-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(explorer): format raw transaction value #2961
Conversation
WalkthroughThe recent changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BridgeTransaction
participant IconAndAmount
participant FormatBigIntToString
User->>BridgeTransaction: Initiate transaction
BridgeTransaction->>IconAndAmount: Pass value instead of formattedValue
IconAndAmount->>FormatBigIntToString: Format bigint value
FormatBigIntToString-->>IconAndAmount: Return formatted string
IconAndAmount-->>BridgeTransaction: Render updated values
BridgeTransaction-->>User: Display transaction details
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This pull request focuses on updating the handling of raw transaction values in the Explorer UI.
packages/explorer-ui/components/BridgeTransaction/BridgeTransactionTable.tsx
: ChangedIconAndAmount
prop fromformattedValue
tovalue
for consistency.packages/explorer-ui/components/misc/IconAndAmount.tsx
: Refactored to useformatBigIntToString
for value formatting, improving readability.packages/explorer-ui/pages/tx/[kappa].tsx
: UpdatedBridgeTransaction
to pass raw values toIconAndAmount
, ensuring consistent formatting.packages/explorer-ui/patch.ts
: AddedBigInt.prototype.toJSON
method for safe JSON serialization ofBigInt
values.packages/explorer-ui/tsconfig.json
: Updated TypeScript target toES2020
for modern JavaScript feature support.packages/explorer-ui/utils/formatBigIntToString.ts
: Introduced utility function for consistentBigInt
value formatting across the codebase.
6 file(s) reviewed, 12 comment(s)
Edit PR Review Bot Settings
@@ -58,7 +58,7 @@ export const BridgeTransactionTable = ({ queryResult }) => { | |||
useExplorerLink={false} | |||
/> | |||
<IconAndAmount | |||
formattedValue={fromInfo.formattedValue} | |||
value={fromInfo.value} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check: Ensure value
is correctly formatted and sanitized before passing to IconAndAmount
.
formattedValue={ | ||
pending ? fromInfo.formattedValue : toInfo?.formattedValue | ||
} | ||
value={pending ? fromInfo.value : toInfo?.value} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Verify that value
handles both pending and confirmed states correctly.
const decimals = addressToDecimals({ tokenAddress, chainId }) | ||
const formattedValue = formatBigIntToString(value, decimals) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Ensure addressToDecimals
handles all edge cases and returns valid decimals.
|
||
const displayAmount = amount ? formatAmount(amount) : '< 0.001' | ||
const decimals = addressToDecimals({ tokenAddress, chainId }) | ||
const formattedValue = formatBigIntToString(value, decimals) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check: Verify formatBigIntToString
correctly formats values for all possible inputs.
@@ -153,7 +153,7 @@ export const BridgeTransaction = ({ queryResult }) => { | |||
<p className="w-24 text-white text-opacity-60">Sent</p> | |||
<div className="flex flex-col items-center sm:flex-row"> | |||
<IconAndAmount | |||
formattedValue={fromInfo.formattedValue} | |||
value={fromInfo.value} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Ensure IconAndAmount
correctly formats raw values for display.
BigInt.prototype.toJSON = function (): number { | ||
return this.toString() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Overriding BigInt.prototype.toJSON
may have unintended side effects if other parts of the codebase rely on the default behavior.
@@ -1,6 +1,6 @@ | |||
{ | |||
"compilerOptions": { | |||
"target": "es5", | |||
"target": "ES2020", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Ensure all environments support ES2020 features to avoid runtime errors.
nativePrecision: number, | ||
decimalPlaces?: number | ||
) => { | ||
if (typeof bi !== 'bigint' && !bi) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider adding an explicit return type for better type safety.
nativePrecision: number, | ||
decimalPlaces?: number | ||
) => { | ||
if (typeof bi !== 'bigint' && !bi) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: The condition typeof bi !== 'bigint' && !bi
might not handle all invalid inputs correctly.
|
||
return str | ||
} catch (error) { | ||
console.error(`error`, error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Error handling only logs to the console. Consider more robust error handling or rethrowing the error.
Deploying sanguine-fe with Cloudflare Pages
|
Bundle ReportChanges will increase total bundle size by 359.64kB ⬆️
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (3)
packages/explorer-ui/patch.ts (1)
1-2
: Consider removing the ESLint and TypeScript ignore comments.These comments disable important linting and type checking, which can hide potential issues. Ensure that the code works without these comments.
packages/explorer-ui/components/misc/IconAndAmount.tsx (1)
5-5
: Ensure theformatBigIntToString
utility function is thoroughly tested.The
formatBigIntToString
function is critical for formatting values correctly. However, no unit tests for this function were found in the codebase. Please add comprehensive unit tests to ensure its reliability.
- No unit tests found for
formatBigIntToString
.Analysis chain
Ensure the
formatBigIntToString
utility function is thoroughly tested.The new utility function is critical for formatting values correctly. Ensure it has comprehensive unit tests.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and coverage of unit tests for the `formatBigIntToString` utility function. # Test: Search for test files related to `formatBigIntToString`. Expect: Comprehensive unit tests. fd formatBigIntToString --type f --extension ts --exec rg 'test'Length of output: 64
Script:
#!/bin/bash # Description: Verify the existence and coverage of unit tests for the `formatBigIntToString` utility function. # Search for any TypeScript test files that import `formatBigIntToString` fd . --type f --extension ts --exec rg 'import .*formatBigIntToString' --context 10Length of output: 4640
Script:
#!/bin/bash # Description: Verify the existence and coverage of unit tests for the `formatBigIntToString` utility function. # Search for any test files that import `formatBigIntToString` and contain test cases. fd . --type f --extension ts --exec rg 'import .*formatBigIntToString' --context 10 | rg 'describe|it|test'Length of output: 508
Script:
#!/bin/bash # Description: Verify the existence and coverage of unit tests for the `formatBigIntToString` utility function. # Search within common test directories for any mention of `formatBigIntToString`. fd formatBigIntToString --type f --extension ts --search-path tests,__tests__,specLength of output: 192
Script:
#!/bin/bash # Description: Verify the existence and coverage of unit tests for the `formatBigIntToString` utility function. # Search for any TypeScript files that contain `formatBigIntToString` and test-related keywords. fd . --type f --extension ts --exec rg 'formatBigIntToString' --context 10 | rg 'describe|it|test'Length of output: 749
packages/explorer-ui/utils/formatBigIntToString.ts (1)
1-5
: Ensure the function parameters are well-documented.Document the parameters and their expected types and values for better readability and maintainability.
/** * Converts a bigint to a string representation with specified decimal places. * * @param {bigint} bi - The bigint to convert. * @param {number} nativePrecision - The native precision of the bigint. * @param {number} [decimalPlaces] - The number of decimal places to include in the output. * @returns {string} The formatted string representation of the bigint. */ export const formatBigIntToString = ( bi: bigint, nativePrecision: number, decimalPlaces?: number ) => {
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (6)
- packages/explorer-ui/components/BridgeTransaction/BridgeTransactionTable.tsx (2 hunks)
- packages/explorer-ui/components/misc/IconAndAmount.tsx (1 hunks)
- packages/explorer-ui/pages/tx/[kappa].tsx (2 hunks)
- packages/explorer-ui/patch.ts (1 hunks)
- packages/explorer-ui/tsconfig.json (2 hunks)
- packages/explorer-ui/utils/formatBigIntToString.ts (1 hunks)
Additional comments not posted (9)
packages/explorer-ui/patch.ts (1)
3-5
: Ensure thetoJSON
method override is necessary and safe.Overriding built-in prototypes can have far-reaching effects. Ensure this change is well-documented and tested, and that it does not introduce unexpected behavior in the codebase.
Verification successful
The
toJSON
method override forBigInt
is safe.The override does not introduce any unexpected behavior in the codebase, as there are no instances of
BigInt
being serialized to JSON in the files that useJSON.stringify
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of BigInt and ensure that the toJSON method override does not introduce issues. # Test: Search for the usage of BigInt in the codebase. Expect: No unexpected behavior due to the toJSON method override. rg --type javascript --type typescript 'BigInt'Length of output: 82
Script:
#!/bin/bash # Description: Verify the usage of BigInt and ensure that the toJSON method override does not introduce issues. # Test: Search for the usage of BigInt in the codebase. Expect: No unexpected behavior due to the toJSON method override. rg --type js --type ts 'BigInt'Length of output: 40366
Script:
#!/bin/bash # Description: Verify if JSON serialization methods like JSON.stringify are used with BigInt instances to ensure the toJSON method override does not introduce issues. # Test: Search for the usage of JSON.stringify in the codebase. Expect: Identify potential impacts of the toJSON method override. rg --type js --type ts 'JSON.stringify'Length of output: 1535
Script:
#!/bin/bash # Description: Verify if JSON serialization methods like JSON.stringify are used with BigInt instances to ensure the toJSON method override does not introduce issues. # Test: Search for the usage of BigInt in the files that contain JSON.stringify. files_with_json_stringify=$(rg --type js --type ts --files-with-matches 'JSON.stringify') if [ -n "$files_with_json_stringify" ]; then rg --type js --type ts 'BigInt' $files_with_json_stringify fiLength of output: 1799
packages/explorer-ui/components/misc/IconAndAmount.tsx (2)
27-31
: Ensure the tooltip content and displayed amount are accurate.The tooltip and displayed amount now derive from the
formattedValue
. Verify that this change does not introduce any discrepancies.Verification successful
Tooltip content and displayed amount verification successful.
The
formattedValue
is consistently used for both the tooltip content and the displayed amount without any discrepancies.
data-tooltip-content={formattedValue}
{formatAmount(formattedValue)}
Both references ensure that the displayed amount and the tooltip content are accurate.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the accuracy of tooltip content and displayed amount. # Test: Search for the usage of `data-tooltip-content` and `formatAmount` to ensure accuracy. Expect: No discrepancies. rg --type javascript --type typescript 'data-tooltip-content' -A 5 rg --type javascript --type typescript 'formatAmount' -A 5Length of output: 195
Script:
#!/bin/bash # Description: Verify the accuracy of tooltip content and displayed amount. # Test: Search for the usage of `data-tooltip-content` and `formatAmount` to ensure accuracy. Expect: No discrepancies. rg --type js --type ts 'data-tooltip-content' -A 5 rg --type js --type ts 'formatAmount' -A 5Length of output: 18691
8-16
: LGTM! Ensure the newvalue
parameter is correctly used across the codebase.The refactoring looks good and simplifies the logic. Verify that all instances of the
IconAndAmount
component use the newvalue
parameter correctly.Verification successful
The new
value
parameter is correctly used across the codebase.The instances of the
IconAndAmount
component in the following files correctly pass thevalue
parameter:
packages/explorer-ui/pages/tx/[kappa].tsx
packages/explorer-ui/components/BridgeTransaction/BridgeTransactionTable.tsx
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `IconAndAmount` component to ensure the new `value` parameter is used correctly. # Test: Search for the usage of `IconAndAmount` in the codebase. Expect: Correct usage of the `value` parameter. rg --type javascript --type typescript 'IconAndAmount'Length of output: 89
Script:
#!/bin/bash # Description: Verify the usage of the `IconAndAmount` component to ensure the new `value` parameter is used correctly. # Test: Search for the usage of `IconAndAmount` in the codebase. Expect: Correct usage of the `value` parameter. rg --type js --type ts 'IconAndAmount'Length of output: 734
Script:
#!/bin/bash # Extract and review the relevant lines of code around the instances of `IconAndAmount` found in the specified files. # Display lines around the found instances in `packages/explorer-ui/pages/tx/[kappa].tsx` rg --type js --type ts -A 5 -B 5 'IconAndAmount' packages/explorer-ui/pages/tx/[kappa].tsx # Display lines around the found instances in `packages/explorer-ui/components/BridgeTransaction/BridgeTransactionTable.tsx` rg --type js --type ts -A 5 -B 5 'IconAndAmount' packages/explorer-ui/components/BridgeTransaction/BridgeTransactionTable.tsxLength of output: 3230
packages/explorer-ui/utils/formatBigIntToString.ts (1)
6-8
: Simplify the type check forbi
.The type check for
bi
can be simplified to improve readability.- if (typeof bi !== 'bigint' && !bi) { + if (typeof bi !== 'bigint') {Likely invalid or redundant comment.
packages/explorer-ui/tsconfig.json (1)
3-3
: Upgrade to ES2020 target approved.The change to
"target": "ES2020"
allows the use of modern JavaScript features and improves performance. Ensure that all environments where this code runs support ES2020.packages/explorer-ui/components/BridgeTransaction/BridgeTransactionTable.tsx (2)
86-86
: Change to raw value approved.The change from
formattedValue
tovalue
simplifies data handling. Ensure that theIconAndAmount
component correctly processes raw values.Verification successful
Change to raw value approved.
The
IconAndAmount
component correctly processes raw values by formatting them internally. The change fromformattedValue
tovalue
in theBridgeTransactionTable.tsx
file is handled appropriately.
IconAndAmount
component implementation:packages/explorer-ui/components/misc/IconAndAmount.tsx
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify handling of raw values in IconAndAmount component. # Test: Search for the IconAndAmount component usage. Expect: Correct handling of raw values. rg --type tsx -A 5 $'IconAndAmount'Length of output: 62
Script:
#!/bin/bash # Description: Verify handling of raw values in IconAndAmount component. # Test: Search for the IconAndAmount component usage. Expect: Correct handling of raw values. rg --type-add 'tsx:*.tsx' -A 5 'IconAndAmount'Length of output: 4182
Script:
#!/bin/bash # Description: Retrieve the implementation details of the IconAndAmount component to verify the handling of the value prop. # Test: Search for the implementation of the IconAndAmount component. ast-grep --lang tsx --pattern $'export const IconAndAmount = ({ value, tokenAddress, chainId, tokenSymbol, iconSize = "w-4 h-4 rounded-full" }) => { $$$ }' --debugLength of output: 384
Script:
#!/bin/bash # Description: Retrieve the implementation details of the IconAndAmount component to verify the handling of the value prop. # Test: Search for the implementation of the IconAndAmount component. ast-grep --lang tsx --pattern $'export const IconAndAmount = ({ value, tokenAddress, chainId, tokenSymbol, iconSize = "w-4 h-4 rounded-full" }) => { $$$ }' --debug-queryLength of output: 367
Script:
#!/bin/bash # Description: Retrieve the implementation details of the IconAndAmount component to verify the handling of the value prop. # Test: Search for the implementation of the IconAndAmount component in the specific file. rg 'export const IconAndAmount' packages/explorer-ui/components/misc/IconAndAmount.tsx -A 20Length of output: 736
61-61
: Change to raw value approved.The change from
formattedValue
tovalue
simplifies data handling. Ensure that theIconAndAmount
component correctly processes raw values.Verification successful
Change to raw value approved.
The
IconAndAmount
component correctly processes raw values by formatting them internally. The change fromformattedValue
tovalue
in theBridgeTransactionTable.tsx
file is handled properly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify handling of raw values in IconAndAmount component. # Test: Search for the IconAndAmount component usage. Expect: Correct handling of raw values. rg --type tsx -A 5 $'IconAndAmount'Length of output: 62
Script:
#!/bin/bash # Description: Verify handling of raw values in IconAndAmount component. # Find all .tsx files and search for the IconAndAmount component definition and usage fd -e tsx -x rg 'IconAndAmount' {}Length of output: 320
Script:
#!/bin/bash # Description: Verify handling of raw values in IconAndAmount component. # Search for the implementation of IconAndAmount in the specified file rg 'IconAndAmount' @components/misc/IconAndAmountLength of output: 120
Script:
#!/bin/bash # Description: Locate and verify handling of raw values in IconAndAmount component. # Find files named IconAndAmount and inspect their implementation fd IconAndAmount -e tsx -e ts -e jsx -e jsLength of output: 99
Script:
#!/bin/bash # Description: Verify handling of raw values in IconAndAmount component. # Search for the implementation of IconAndAmount in the specified file rg 'IconAndAmount' packages/explorer-ui/components/misc/IconAndAmount.tsx -A 20Length of output: 721
packages/explorer-ui/pages/tx/[kappa].tsx (2)
156-156
: Change to raw value approved.The change from
formattedValue
tovalue
simplifies data handling. Ensure that theIconAndAmount
component correctly processes raw values.Verification successful
Handling of raw values in IconAndAmount component is correct.
The
IconAndAmount
component processes the rawvalue
by formatting it before rendering, ensuring proper display.
packages/explorer-ui/components/misc/IconAndAmount.tsx
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify handling of raw values in IconAndAmount component. # Test: Search for the IconAndAmount component usage. Expect: Correct handling of raw values. rg --type tsx -A 5 $'IconAndAmount'Length of output: 62
Script:
#!/bin/bash # Description: Verify handling of raw values in IconAndAmount component. # Search for the definition of the IconAndAmount component rg 'function IconAndAmount' -A 10 # Search for the usage of the IconAndAmount component rg 'IconAndAmount' -A 5Length of output: 4195
Script:
#!/bin/bash # Description: Retrieve the full implementation of the IconAndAmount component. # Search for the full implementation of the IconAndAmount component rg 'export const IconAndAmount' -A 50 packages/explorer-ui/components/misc/IconAndAmount.tsxLength of output: 1036
182-182
: Change to raw value approved.The change from
formattedValue
tovalue
simplifies data handling. Ensure that theIconAndAmount
component correctly processes raw values.Verification successful
Change to raw value approved.
The
IconAndAmount
component correctly processes raw values by formatting them internally. The change fromformattedValue
tovalue
simplifies data handling without issues.
packages/explorer-ui/components/misc/IconAndAmount.tsx
: Thevalue
prop is formatted usingformatBigIntToString(value, decimals)
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify handling of raw values in IconAndAmount component. # Test: Search for the IconAndAmount component usage. Expect: Correct handling of raw values. rg --type tsx -A 5 $'IconAndAmount'Length of output: 62
Script:
#!/bin/bash # Description: Verify handling of raw values in IconAndAmount component. # Test: Search for the IconAndAmount component usage. Expect: Correct handling of raw values. rg -A 5 'IconAndAmount'Length of output: 4159
Script:
#!/bin/bash # Description: Extract the implementation of IconAndAmount component to verify handling of raw values. # Test: Extract the IconAndAmount component implementation. ast-grep --lang tsx --pattern $'export const IconAndAmount = ({ value, tokenAddress, chainId, tokenSymbol, iconSize }) => { $$$ }' packages/explorer-ui/components/misc/IconAndAmount.tsxLength of output: 188
Script:
#!/bin/bash # Description: Extract the implementation of IconAndAmount component to verify handling of raw values. # Test: Extract the IconAndAmount component implementation. ast-grep --lang tsx --pattern 'export const IconAndAmount = ({ value, $_ }) => { $$$ }' packages/explorer-ui/components/misc/IconAndAmount.tsxLength of output: 146
Script:
#!/bin/bash # Description: Verify handling of raw values in IconAndAmount component. # Test: Search for the usage of the value prop in IconAndAmount component's file. rg 'value' packages/explorer-ui/components/misc/IconAndAmount.tsxLength of output: 137
} catch (error) { | ||
console.error(`error`, error) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle errors more gracefully.
Instead of logging errors to the console, consider throwing an error or handling it in a way that the calling function can manage.
- console.error(`error`, error)
+ throw new Error(`Failed to format bigint: ${error.message}`)
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
} catch (error) { | |
console.error(`error`, error) | |
} | |
} catch (error) { | |
throw new Error(`Failed to format bigint: ${error.message}`) | |
} |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2961 +/- ##
=============================================
Coverage 25.70725% 25.70725%
=============================================
Files 771 771
Lines 55603 55603
Branches 80 80
=============================================
Hits 14294 14294
Misses 39823 39823
Partials 1486 1486
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
New Features
BridgeTransactionTable
andIconAndAmount
components for improved value representation.BigInt
objects, improving compatibility with JSON data consumption.bigint
values into a formatted string representation, enhancing numerical display options.Bug Fixes
formattedValue
in theIconAndAmount
component, improving clarity and reducing potential errors.Chores
f633c7a: explorer-ui preview link