fix constants publish [SLT-160] #3224
Conversation
trajan0x
requested review from
bigboydiamonds and
abtestingalpha
as code owners
WalkthroughThe pull request involves changes to the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://github.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
packages/synapse-constants/package.json (2)
3-3: Consider updating the version number.The package name has been changed, which is a significant modification. However, the version number remains unchanged at 1.5.6. It's generally a good practice to update the version number when making notable changes to a package.
Consider incrementing the version number according to semantic versioning principles. For example:
- "version": "1.5.6", + "version": "2.0.0",This would indicate a major change due to the potential breaking change in the package name.
Line range hint
28-29: Inconsistent use of package managers in scripts.The
prepublishscript uses yarn, while other scripts use npm. This inconsistency could lead to issues, especially in CI/CD pipelines or when different team members use different package managers.Consider standardizing on one package manager throughout the scripts. For example:
- "prepublish": "yarn build", + "prepublish": "npm run build",Alternatively, if yarn is preferred, update other scripts to use yarn for consistency.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- packages/explorer-ui/package.json (1 hunks)
- packages/synapse-constants/package.json (1 hunks)
🔇 Additional comments (2)
packages/explorer-ui/package.json (2)
Line range hint
3-3: Version bump to 0.3.8The package version has been updated from the previous version. This minor version bump suggests new features or non-breaking changes have been introduced.
Line range hint
1-95: Summary of changesThe changes in this file are minimal and focused:
- The package version has been updated to 0.3.8.
- The
synapse-constantsdependency has been renamed to@synapsecns/synapse-constants.These changes align with the PR objectives of fixing the constants publish issue. The dependency rename suggests a namespace update, which could potentially affect imports across the project. Ensure that all relevant files have been updated to use the new package name.
Bundle ReportChanges will decrease total bundle size by 10.11MB (-24.54%) ⬇️. This is within the configured threshold ✅ Detailed changes
ℹ️ *Bundle size includes cached data from a previous commit |
Deploying sanguine-fe with
|
| Latest commit: |
0f2b4a0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://83a3ea07.sanguine-fe.pages.dev |
| Branch Preview URL: | https://fix-synapse-constants.sanguine-fe.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
- docs/bridge/package.json (1 hunks)
github-actions
bot
added
javascript
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3224 +/- ##
=============================================
Coverage 90.43902% 90.43902%
=============================================
Files 54 54
Lines 1025 1025
Branches 82 82
=============================================
Hits 927 927
Misses 95 95
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
docs/bridge/src/components/USDC.tsx (1)
Line range hint
2-7: Consider removing console.log statementsThe
console.logstatements on lines 2 and 5 are likely used for debugging purposes. Consider removing or commenting out these statements before merging to production, as they may clutter the console in a live environment.You can apply the following changes:
-console.log(0, USDC) +// console.log(0, USDC) export const Test = () => { - console.log(1, USDC.icon) + // console.log(1, USDC.icon) return <USDC.icon /> }The usage of
USDC.iconin theTestcomponent looks correct and doesn't require any changes.packages/explorer-ui/components/misc/ChainImage.tsx (1)
Line range hint
7-24: Consider adding error handling for invalid chainIdThe current implementation doesn't handle cases where
chainIdis invalid or not found inCHAINS_BY_ID. To improve the component's robustness, consider adding a fallback for such scenarios.Here's a suggested improvement:
export const ChainImage = ({ chainId, imgSize = 'w-4 h-4', className }) => { if (chainId) { const chain = CHAINS_BY_ID[chainId] + if (!chain) { + console.warn(`Invalid chainId: ${chainId}`); + return ( + <QuestionMarkCircleIcon + className={`${imgSize} rounded-full mr-2 inline ${className}`} + strokeWidth={1} + /> + ); + } return ( <Image src={chain.chainImg} className={`${imgSize} rounded-full mr-2 inline ${className}`} alt={chain.name} height={16} width={16} /> ) } else { return ( <QuestionMarkCircleIcon - className={`${imgSize} rounded-full mr-2 inline`} + className={`${imgSize} rounded-full mr-2 inline ${className}`} strokeWidth={1} /> ) } }This change adds error handling for invalid
chainIdand ensures consistent use of theclassNameprop.packages/explorer-ui/utils/classes/Token.ts (1)
Line range hint
1-144: Summary of changes and recommendationsThe main change in this file is the update of the import statement for CHAINS. While this change appears to be part of a larger refactoring effort to use a scoped package, it's crucial to ensure that this modification doesn't introduce any unintended side effects.
Recommendations:
- Verify the consistency of this import change across the entire project.
- Ensure that the ChainId object derived from CHAINS still contains all expected properties.
- Test the behavior of the makeMultiChainObj function to confirm it works correctly with the new import.
- Update any project documentation or README files to reflect this package change.
These steps will help maintain the integrity of the Token class and related functionality while adopting the new package structure.
packages/explorer-ui/components/pages/Home/index.tsx (1)
Line range hint
32-33: Revert state setter naming to maintain consistencyThe renaming of
setDailyStatisticDurationtoSetDailyStatisticDurationandsetDailyStatisticCumulativetoSetDailyStatisticCumulativeintroduces inconsistency in the naming convention for state setters. React convention typically uses camelCase for state setters.Please revert these changes to maintain consistency with React conventions and the rest of the codebase. Apply the following diff:
- const [dailyStatisticDuration, SetDailyStatisticDuration] = + const [dailyStatisticDuration, setDailyStatisticDuration] = useState('PAST_6_MONTHS') - const [dailyStatisticCumulative, SetDailyStatisticCumulative] = useState(true) + const [dailyStatisticCumulative, setDailyStatisticCumulative] = useState(true) // ... (other code) - onClick={() => SetDailyStatisticDuration('PAST_MONTH')} + onClick={() => setDailyStatisticDuration('PAST_MONTH')} // ... (other code) - onClick={() => SetDailyStatisticDuration('PAST_3_MONTHS')} + onClick={() => setDailyStatisticDuration('PAST_3_MONTHS')} // ... (other code) - onClick={() => SetDailyStatisticDuration('PAST_6_MONTHS')} + onClick={() => setDailyStatisticDuration('PAST_6_MONTHS')} // ... (other code) - onClick={() => SetDailyStatisticCumulative(false)} + onClick={() => setDailyStatisticCumulative(false)} // ... (other code) - onClick={() => SetDailyStatisticCumulative(true)} + onClick={() => setDailyStatisticCumulative(true)}Also applies to: 36-37, 201-202, 205-206, 226-227, 230-231
packages/explorer-ui/utils/styles/networks.ts (1)
Line range hint
1-438: Consider refactoring for improved maintainabilityWhile the current implementation is functional, consider the following improvements:
- Reduce duplication by abstracting common patterns in the utility functions.
- Use a centralized theme configuration for colors instead of hardcoding them in each function.
- Consider using a map or object for color lookups instead of switch statements to improve readability and maintainability.
Example refactoring:
const NETWORK_THEME = { yellow: { text: '[#ecae0b]', bg: 'stone-800', // ... other properties }, // ... other network themes }; const getNetworkStyle = (chainId, styleType) => { const color = NETWORK_COLORS[chainId]; const theme = NETWORK_THEME[color]; return theme ? theme[styleType] : ''; }; export const getNetworkTextColor = (chainId) => `text-${getNetworkStyle(chainId, 'text')}`; export const getNetworkBgClassName = (chainId) => `bg-${getNetworkStyle(chainId, 'bg')}`; // ... other utility functionsThis refactoring would make the code more maintainable and easier to update in the future.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (21)
- docs/bridge/src/components/Routes.tsx (1 hunks)
- docs/bridge/src/components/USDC.tsx (1 hunks)
- packages/explorer-ui/components/misc/AssetImage.tsx (1 hunks)
- packages/explorer-ui/components/misc/ChainImage.tsx (1 hunks)
- packages/explorer-ui/components/misc/ChainInfo.tsx (1 hunks)
- packages/explorer-ui/components/misc/MostActive.tsx (1 hunks)
- packages/explorer-ui/components/misc/TokenOnChain.tsx (1 hunks)
- packages/explorer-ui/components/misc/ToolTip.tsx (1 hunks)
- packages/explorer-ui/components/pages/Home/UniversalSearch/index.tsx (1 hunks)
- packages/explorer-ui/components/pages/Home/index.tsx (1 hunks)
- packages/explorer-ui/pages/chain/[chainId].tsx (1 hunks)
- packages/explorer-ui/pages/token/[tokenAddress].tsx (1 hunks)
- packages/explorer-ui/pages/tx/[kappa].tsx (2 hunks)
- packages/explorer-ui/pages/txs/index.tsx (1 hunks)
- packages/explorer-ui/utils/addressToDecimals.ts (1 hunks)
- packages/explorer-ui/utils/addressToSymbol.ts (1 hunks)
- packages/explorer-ui/utils/classes/Token.ts (1 hunks)
- packages/explorer-ui/utils/styles/coins.ts (1 hunks)
- packages/explorer-ui/utils/styles/networks.ts (1 hunks)
- packages/explorer-ui/utils/urls/index.ts (1 hunks)
- packages/synapse-constants/rollup.config.js (1 hunks)
✅ Files skipped from review due to trivial changes (9)
- docs/bridge/src/components/Routes.tsx
- packages/explorer-ui/components/misc/TokenOnChain.tsx
- packages/explorer-ui/components/pages/Home/UniversalSearch/index.tsx
- packages/explorer-ui/pages/chain/[chainId].tsx
- packages/explorer-ui/pages/txs/index.tsx
- packages/explorer-ui/utils/addressToDecimals.ts
- packages/explorer-ui/utils/addressToSymbol.ts
- packages/explorer-ui/utils/styles/coins.ts
- packages/explorer-ui/utils/urls/index.ts
🔇 Additional comments (21)
docs/bridge/src/components/USDC.tsx (1)
1-1: LGTM! Verify consistency across the project.The change from
'synapse-constants'to'@synapsecns/synapse-constants'aligns with the PR objective to fix the constants publish workflow. This update to use a scoped package name is likely part of the solution.To ensure consistency, please verify that this change has been applied across all files in the project. Run the following script to check for any remaining imports from the old package name:
If the script returns any results, those files may need to be updated as well.
✅ Verification successful
Verified! All imports from
'synapse-constants'have been successfully updated to'@synapsecns/synapse-constants'across the project. No remaining instances of the old import were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining imports from 'synapse-constants' # Test: Search for imports from 'synapse-constants'. Expect: No results. rg --type typescript --type javascript "from 'synapse-constants'"Length of output: 108
Script:
#!/bin/bash # Description: Check for any remaining imports from 'synapse-constants' # Test: Search for imports from 'synapse-constants' in .ts, .tsx, and .js files. Expect: No results. rg "from 'synapse-constants'" --glob '*.ts' --glob '*.tsx' --glob '*.js'Length of output: 80
packages/explorer-ui/components/misc/ChainImage.tsx (1)
2-2: LGTM! Verify the package availability.The import statement has been updated to use the scoped package
@synapsecns/synapse-constants, which aligns with the PR objective to fix the constants publish issue. This change should resolve the broken publishing workflow.To ensure the package is correctly published and accessible, please run the following verification script:
packages/synapse-constants/rollup.config.js (1)
44-44: Bundle name updated to reflect scoped package format.The change from
'synapse-constants'to'@synapsecns/synapse-constants'aligns the bundle name with the scoped package format. This is likely the correct fix for the broken publishing workflow mentioned in the PR objectives.To ensure consistency across the project, let's verify if this change is reflected in other relevant files:
This script will help us verify that the package name has been updated consistently across the project.
packages/explorer-ui/components/misc/MostActive.tsx (2)
Line range hint
1-52: Summary: Import change looks good, no further modifications needed.The only change in this file is updating the import statement to use the scoped package
@synapsecns/synapse-constants. The rest of the file, including the usage ofTOKEN_HASH_MAPandCHAINS, remains unchanged and consistent with the new import. This change appears to be part of a larger refactoring effort to improve package management.
1-1: LGTM! Verify package consistency across the project.The import statement has been updated to use the scoped package
@synapsecns/synapse-constants. This change looks good and likely reflects an improvement in package management or namespace organization.To ensure consistency across the project, please run the following script:
This script will help identify any inconsistencies in import statements across the project.
✅ Verification successful
Update Verification Script to Correct File Type Specifications
The previous script encountered errors due to unrecognized file types. Here's the revised script targeting the correct file extensions for TypeScript and JavaScript:
This updated script uses file extensions to accurately search for import statements, ensuring reliable verification across the project.
Import Statement Updated Consistently Across the Project
All imports from
'synapse-constants'have been successfully replaced with@synapsecns/synapse-constantsacross the codebase. No remaining instances of the old import were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of @synapsecns/synapse-constants across the project # Test: Check for any remaining imports from 'synapse-constants' echo "Checking for any remaining imports from 'synapse-constants':" rg --type typescript --type javascript "from 'synapse-constants'" -g '!**/node_modules/**' # Test: Verify consistent usage of @synapsecns/synapse-constants echo "Verifying consistent usage of @synapsecns/synapse-constants:" rg --type typescript --type javascript "from '@synapsecns/synapse-constants'" -g '!**/node_modules/**'Length of output: 547
Script:
#!/bin/bash # Description: Verify the usage of @synapsecns/synapse-constants across the project # Test: Check for any remaining imports from 'synapse-constants' echo "Checking for any remaining imports from 'synapse-constants':" rg "from 'synapse-constants'" -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' -g '!**/node_modules/**' # Test: Verify consistent usage of @synapsecns/synapse-constants echo "Verifying consistent usage of @synapsecns/synapse-constants:" rg "from '@synapsecns/synapse-constants'" -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' -g '!**/node_modules/**'Length of output: 2623
packages/explorer-ui/components/misc/ChainInfo.tsx (1)
3-3: LGTM! Verify similar imports across the project.The import statement has been correctly updated to use the scoped package
@synapsecns/synapse-constants. This change aligns with the PR objective to fix the constants publish workflow.To ensure consistency across the project, please run the following script to check for any remaining imports from 'synapse-constants':
packages/explorer-ui/components/misc/AssetImage.tsx (2)
Line range hint
5-65: Consistent usage of imported constantsThe usage of the imported constants (TOKEN_HASH_MAP, AVWETH, USDC) remains consistent throughout the file. No changes were required in the component logic or the helper function due to the import path update.
2-2: Approve import path change and suggest verificationThe import path has been updated from 'synapse-constants' to '@synapsecns/synapse-constants', which aligns with the PR objective of fixing the constants publish workflow. This change looks good and should resolve the publishing issue.
To ensure consistency across the project, please run the following script to check for any remaining instances of the old import path:
If the script returns any results, those files may need to be updated as well.
packages/explorer-ui/components/misc/ToolTip.tsx (1)
2-2: LGTM! Verify impact on other files.The import statement has been correctly updated to use the new package name
@synapsecns/synapse-constants. This change aligns with the PR objective of fixing the constants publish workflow.To ensure consistency across the project, please run the following script to check for any remaining imports from the old package name:
If the script returns any results, those files will need to be updated as well.
✅ Verification successful
Verified: No remaining imports from 'synapse-constants'.
All import statements have been successfully updated to use
@synapsecns/synapse-constants. No further imports from the old package were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining imports from 'synapse-constants' # Test: Search for imports from 'synapse-constants'. Expect: No results. rg --type typescript --type javascript "from 'synapse-constants'" -g '!**/node_modules/**'Length of output: 133
Script:
#!/bin/bash # Description: Check for any remaining imports from 'synapse-constants' # Test: Search for imports from 'synapse-constants'. Expect: No results. rg "from 'synapse-constants'" --glob '!**/node_modules/**' --glob '**/*.ts' --glob '**/*.tsx' --glob '**/*.js' --glob '**/*.jsx'Length of output: 136
Script:
#!/bin/bash # Description: Verify no remaining imports from 'synapse-constants' # Find all relevant files excluding node_modules and search for the import statement fd --extension ts --extension tsx --extension js --extension jsx --exclude node_modules | xargs grep -H "from ['\"]synapse-constants['\"]"Length of output: 144
packages/explorer-ui/utils/classes/Token.ts (3)
4-4: Verify ChainId object structureThe change in the import statement shouldn't affect the Token class functionality. However, it's crucial to ensure that the ChainId object derived from CHAINS still contains the expected properties.
Please run the following script to verify the ChainId object structure:
#!/bin/bash # Description: Verify the ChainId object structure # Test: Search for ChainId usage and log its structure ast-grep --lang typescript --pattern 'const ChainId = CHAINS.ChainId' echo "\nPlease manually verify that the ChainId object still contains the expected properties in the new @synapsecns/synapse-constants package."
Line range hint
135-144: Verify makeMultiChainObj function behaviorThe makeMultiChainObj function's logic remains unchanged. However, since it relies on the ChainId object, which is derived from the newly imported CHAINS, it's important to verify that the function still behaves as expected.
Please run the following test to ensure the function's behavior remains consistent:
#!/bin/bash # Description: Verify makeMultiChainObj function behavior # Test: Search for makeMultiChainObj function usage rg --type typescript "makeMultiChainObj\(" -A 5 echo "\nPlease manually verify that all usages of makeMultiChainObj function still produce the expected results with the new @synapsecns/synapse-constants package."
2-2: Approve import change and verify consistencyThe update to the import statement from 'synapse-constants' to '@synapsecns/synapse-constants' looks good. This change to a scoped package can improve package management.
To ensure consistency across the project, please run the following script:
packages/explorer-ui/pages/token/[tokenAddress].tsx (3)
Line range hint
134-136: Usage of TOKEN_HASH_MAP remains correct.The usage of
TOKEN_HASH_MAPin the component remains unchanged and correct after the import path update. It's properly accessed using thetokenChainIDandaddressvariables.
Line range hint
1-236: Summary: Minor import change with potential project-wide implications.The only significant change in this file is the update to the import statement for
TOKEN_HASH_MAP. The rest of the file, including theTokencomponent implementation, remains unchanged.While this change doesn't affect the functionality of this specific component, it suggests a broader update in the project structure or package management.
To ensure the consistency and correctness of this change:
- Verify that this import change is part of a planned update across the project.
- Check if similar changes are needed in other files importing from 'synapse-constants'.
- Ensure that the build and deployment processes are updated to accommodate this change in import structure.
Run the following script to identify other files that might need similar updates:
#!/bin/bash # Description: Identify files that might need similar import updates # Search for files still using the old import pattern echo "Files potentially needing import updates:" rg --type typescript --type javascript "from\s+['\"]synapse-constants['\"]"This will help identify any files that might need similar import updates to maintain consistency across the project.
5-5: Verify the new import path and ensure consistency.The change from a relative import to an absolute import for
TOKEN_HASH_MAPlooks good. This update suggests a shift in the project structure or package management.Please ensure the following:
- Verify that the new import path
'@synapsecns/synapse-constants'is correct and accessible in the project setup.- Check if this change needs to be applied consistently across other files in the project.
Run the following script to verify the usage of this import across the project:
This will help identify any inconsistencies in the import statements across the project.
✅ Verification successful
Import path change for
TOKEN_HASH_MAPverified and consistent across the project.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for inconsistent imports of TOKEN_HASH_MAP # Test 1: Search for any remaining relative imports of TOKEN_HASH_MAP echo "Checking for remaining relative imports of TOKEN_HASH_MAP:" rg --type typescript --type javascript "from\s+['\"]synapse-constants['\"]" # Test 2: Search for the new absolute import to ensure consistency echo "Checking for the new absolute import of TOKEN_HASH_MAP:" rg --type typescript --type javascript "from\s+['\"]@synapsecns/synapse-constants['\"]"Length of output: 304
Script:
# New shell script to verify import statements correctly #!/bin/bash # Description: Check for inconsistent imports of TOKEN_HASH_MAP # Test 1: Search for any remaining relative imports of TOKEN_HASH_MAP echo "Checking for remaining relative imports of TOKEN_HASH_MAP:" rg --glob '*.ts' --glob '*.tsx' "from\s+['\"]synapse-constants['\"]" # Test 2: Search for the new absolute import to ensure consistency echo "Checking for the new absolute import of TOKEN_HASH_MAP:" rg --glob '*.ts' --glob '*.tsx' "from\s+['\"]@synapsecns/synapse-constants['\"]"Length of output: 2543
Script:
# Revised shell script to verify import statements correctly #!/bin/bash # Description: Check for inconsistent imports of TOKEN_HASH_MAP # Test 1: Search for any remaining relative imports of TOKEN_HASH_MAP echo "Checking for remaining relative imports of TOKEN_HASH_MAP:" rg --type typescript --type typescriptreact "from\s+['\"]synapse-constants['\"]" # Test 2: Search for the new absolute import to ensure consistency echo "Checking for the new absolute import of TOKEN_HASH_MAP:" rg --type typescript --type typescriptreact "from\s+['\"]@synapsecns/synapse-constants['\"]"Length of output: 503
Script:
# Updated shell script to verify import statements using file extensions #!/bin/bash # Description: Check for inconsistent imports of TOKEN_HASH_MAP # Test 1: Search for any remaining relative imports of TOKEN_HASH_MAP echo "Checking for remaining relative imports of TOKEN_HASH_MAP:" rg --glob '*.ts' --glob '*.tsx' "from\s+['\"]synapse-constants['\"]" # Test 2: Search for the new absolute import to ensure consistency echo "Checking for the new absolute import of TOKEN_HASH_MAP:" rg --glob '*.ts' --glob '*.tsx' "from\s+['\"]@synapsecns/synapse-constants['\"]"Length of output: 2543
packages/explorer-ui/pages/tx/[kappa].tsx (3)
Line range hint
1-324: Summary of changes and their impact.The changes made to this file are minor and don't affect the core functionality of the component. They include:
- Updating the import statement for
CHAINS.- Removing semicolons from return statements in the
getTimeElapsedStrfunction.While these changes seem harmless, it's crucial to ensure they align with the project's coding standards and don't introduce any unintended side effects. Please review the verification steps in the previous comments to maintain consistency across the project.
The changes look good overall, but please address the verification steps mentioned in the previous comments to ensure project-wide consistency.
42-46: Verify consistency with coding standards.The semicolons have been removed from the return statements in the
getTimeElapsedStrfunction. While this doesn't affect functionality, it's important to ensure this change aligns with the project's coding standards.Please run the following script to check the consistency of semicolon usage in return statements across the project:
#!/bin/bash # Check for return statements with and without semicolons echo "Return statements with semicolons:" rg "return .+;" --type ts --type tsx echo "Return statements without semicolons:" rg "return .+[^;]$" --type ts --type tsxIf the results show inconsistencies, consider updating the project's linting rules or ESLint configuration to enforce a consistent style for return statements.
7-7: Verify the consistency of the updated import statement.The import statement for
CHAINShas been updated to use@synapsecns/synapse-constants. This change likely reflects an update in the package structure or naming convention.Please run the following script to ensure this change is consistent across the entire project and that the
package.jsonfile has been updated accordingly:If the script finds any remaining imports from 'synapse-constants' or if
@synapsecns/synapse-constantsis not inpackage.json, please update them accordingly.packages/explorer-ui/components/pages/Home/index.tsx (2)
Line range hint
1-385: Overall assessment: Minor changes with room for improvementThe changes in this file are minimal and don't significantly impact the functionality of the
Homecomponent. The import path update forCHAINSis appropriate and aligns with the PR objective. However, the renaming of state setters introduces inconsistency and should be reverted.Apart from these changes, the component's structure, logic, and UI rendering remain intact. The code maintains good practices in terms of component organization, state management, and UI composition.
11-11: LGTM. Verify consistency across the project.The import path update for
CHAINSlooks good and aligns with the PR objective of fixing the constants publish workflow.To ensure consistency, please run the following command to check for any remaining occurrences of the old import path:
If the command returns any results, those files may need to be updated as well.
packages/explorer-ui/utils/styles/networks.ts (1)
1-1: LGTM! Verify impact on other files.The update to a scoped package (
@synapsecns/synapse-constants) is a good practice. Ensure that this change is consistently applied across the entire project for all imports from the previous 'synapse-constants' package.To verify the consistency of this change across the project, run the following script:
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/ui-preview.yaml (1)
66-71: LGTM: Addition of synapse-constants build stepThe new build step for synapse-constants is a good addition to the workflow. It ensures that the package is properly built as part of the deployment process, which is crucial for fixing the constants publish issue.
Some suggestions for potential improvements:
- Consider adding error handling to the yarn commands.
- It might be beneficial to capture and log the output of the build process for debugging purposes.
Here's a suggested improvement to the build step:
- name: Build synapse-constants run: | cd packages/synapse-constants yarn install || { echo "Failed to install dependencies"; exit 1; } yarn build || { echo "Failed to build package"; exit 1; } echo "Successfully built synapse-constants" working-directory: ${{ github.workspace }}This version adds basic error handling and provides more informative output.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
- .github/workflows/ui-preview.yaml (2 hunks)
- docs/bridge/package.json (1 hunks)
- packages/explorer-ui/components/misc/AssetImage.tsx (1 hunks)
- packages/explorer-ui/components/misc/ChainImage.tsx (1 hunks)
- packages/explorer-ui/components/misc/ChainInfo.tsx (1 hunks)
- packages/explorer-ui/components/misc/MostActive.tsx (1 hunks)
- packages/explorer-ui/components/misc/TokenOnChain.tsx (1 hunks)
- packages/explorer-ui/components/misc/ToolTip.tsx (1 hunks)
- packages/explorer-ui/components/pages/Home/UniversalSearch/index.tsx (2 hunks)
- packages/explorer-ui/components/pages/Home/index.tsx (1 hunks)
- packages/explorer-ui/pages/chain/[chainId].tsx (1 hunks)
- packages/explorer-ui/pages/token/[tokenAddress].tsx (1 hunks)
- packages/explorer-ui/pages/tx/[kappa].tsx (2 hunks)
- packages/explorer-ui/pages/txs/index.tsx (1 hunks)
- packages/explorer-ui/utils/addressToDecimals.ts (1 hunks)
- packages/explorer-ui/utils/addressToSymbol.ts (1 hunks)
- packages/explorer-ui/utils/classes/Token.ts (2 hunks)
- packages/explorer-ui/utils/styles/coins.ts (1 hunks)
- packages/explorer-ui/utils/styles/networks.ts (1 hunks)
- packages/explorer-ui/utils/urls/index.ts (1 hunks)
- packages/synapse-constants/package.json (4 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/explorer-ui/pages/token/[tokenAddress].tsx
🚧 Files skipped from review as they are similar to previous changes (19)
- docs/bridge/package.json
- packages/explorer-ui/components/misc/AssetImage.tsx
- packages/explorer-ui/components/misc/ChainImage.tsx
- packages/explorer-ui/components/misc/ChainInfo.tsx
- packages/explorer-ui/components/misc/MostActive.tsx
- packages/explorer-ui/components/misc/TokenOnChain.tsx
- packages/explorer-ui/components/misc/ToolTip.tsx
- packages/explorer-ui/components/pages/Home/UniversalSearch/index.tsx
- packages/explorer-ui/components/pages/Home/index.tsx
- packages/explorer-ui/pages/chain/[chainId].tsx
- packages/explorer-ui/pages/tx/[kappa].tsx
- packages/explorer-ui/pages/txs/index.tsx
- packages/explorer-ui/utils/addressToDecimals.ts
- packages/explorer-ui/utils/addressToSymbol.ts
- packages/explorer-ui/utils/classes/Token.ts
- packages/explorer-ui/utils/styles/coins.ts
- packages/explorer-ui/utils/styles/networks.ts
- packages/explorer-ui/utils/urls/index.ts
- packages/synapse-constants/package.json
🔇 Additional comments (2)
.github/workflows/ui-preview.yaml (2)
11-11: LGTM: Addition of synapse-constants to workflow triggerThe inclusion of 'packages/synapse-constants/**' in the pull_request trigger is appropriate. This change ensures that the workflow runs when changes are made to the synapse-constants package, which aligns with the PR objective of fixing the constants publish process.
Line range hint
1-124: Summary: Changes align well with PR objectivesThe modifications to this workflow file effectively address the PR objective of fixing the constants publish process. The two main changes:
- Adding 'packages/synapse-constants/**' to the pull_request trigger
- Introducing a new build step for synapse-constants
These changes ensure that the synapse-constants package is included in the UI preview deployment process. The modifications are minimal and targeted, which helps maintain the overall integrity of the workflow while addressing the specific issue at hand.
Description
fixes broken publishing workflow
Summary by CodeRabbit
synapse-constantsto@synapsecns/synapse-constants, aligning with a new scoped naming convention.synapse-constantsin the bridge documentation to the new scoped version.0.3.8for the@synapsecns/explorer-uipackage.synapse-constantsin the monorepo configuration.synapse-constantspackage in the GitHub workflow.0d1d7e7: docs preview link
db7521c: docs preview link
17bfd5a: docs preview link
cd89fa5: docs preview link
11c24f3: docs preview link
bfca1eb: docs preview link
4856e6b: docs preview link
4f3a099: docs preview link
a0e6825: docs preview link
bc3bdf0: docs preview link
19bb8da: docs preview link
19bb8da: explorer-ui preview link
982e12e: docs preview link
982e12e: explorer-ui preview link
05307ec: docs preview link
05307ec: explorer-ui preview link
d85945a: docs preview link
d85945a: explorer-ui preview link
d428186: docs preview link
d428186: explorer-ui preview link