-
Notifications
You must be signed in to change notification settings - Fork 72
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: update to starknet.js v6 #846
Conversation
WalkthroughThe updates include version upgrades for Changes
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 Configration File (
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
Files selected for processing (1)
- package.json (1 hunks)
Files skipped from review due to trivial changes (1)
- package.json
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
Files selected for processing (1)
- package.json (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- package.json
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: 2
Outside diff range and nitpick comments (4)
utils/altcoinService.ts (1)
Line range hint
155-158
: Use modern JavaScript exponentiation operator.Replace
Math.pow
with the exponentiation operator**
for better readability and performance.- const strkConvertedBalance = (strkBalance * strkQuote) / BigInt(Math.pow(10, quoteData.decimals)); + const strkConvertedBalance = (strkBalance * strkQuote) / BigInt(10 ** quoteData.decimals);Tools
Biome
[error] 78-85: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
components/identities/actions/clickable/clickablePersonhoodIcon.tsx (1)
Line range hint
93-93
: Implement optional chaining for improved code safety.Switching to optional chaining can prevent runtime errors when accessing properties on potentially null or undefined objects.
- return account?.signMessage(payload as TypedData) as Promise< + return account?.signMessage(payload as TypedData)? as Promise<components/discount/freeRenewalDiscount.tsx (2)
Line range hint
242-242
: Simplify conditional expression.Directly assign the result without using a ternary operator for more concise code:
- setTermsBox(!termsBox) + setTermsBox(!termsBox)Tools
Biome
[error] 288-289: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Line range hint
442-442
: Improve accessibility by adding alternative text to images.It's crucial to provide a text alternative for images to support screen reader users:
- <img className={styles.image} src="/visuals/register.webp" /> + <img className={styles.image} src="/visuals/register.webp" alt="Registration visual" />Tools
Biome
[error] 288-289: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- components/UI/searchBar.tsx (2 hunks)
- components/discount/freeRenewalDiscount.tsx (1 hunks)
- components/identities/actions/clickable/clickablePersonhoodIcon.tsx (2 hunks)
- context/StarknetIdJsProvider.tsx (1 hunks)
- hooks/naming.ts (1 hunks)
- hooks/useAllowanceCheck.tsx (1 hunks)
- hooks/useBalances.tsx (1 hunks)
- hooks/useNeedAllowances.tsx (1 hunks)
- hooks/useNotificationManager.ts (2 hunks)
- types/frontTypes.d.ts (2 hunks)
- utils/altcoinService.ts (1 hunks)
- utils/cacheDomainData.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- hooks/useBalances.tsx
Additional context used
Biome
context/StarknetIdJsProvider.tsx
[error] 27-27: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
utils/altcoinService.ts
[error] 78-85: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 95-100: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 155-155: Use the '**' operator instead of 'Math.pow'.
Unsafe fix: Use the '**' operator instead of 'Math.pow'.
(lint/style/useExponentiationOperator)
[error] 158-158: Use the '**' operator instead of 'Math.pow'.
Unsafe fix: Use the '**' operator instead of 'Math.pow'.
(lint/style/useExponentiationOperator)
components/UI/searchBar.tsx
[error] 192-227: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 199-227: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
components/identities/actions/clickable/clickablePersonhoodIcon.tsx
[error] 93-93: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 154-157: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 181-181: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
[error] 180-180: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
form
element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
components/discount/freeRenewalDiscount.tsx
[error] 242-242: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
[error] 288-289: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 442-442: Provide a text alternative through the alt, aria-label or aria-labelledby attribute
Meaningful alternative text on elements helps users relying on screen readers to understand content's purpose within a page.
(lint/a11y/useAltText)
Additional comments not posted (8)
hooks/useAllowanceCheck.tsx (1)
27-32
: Type casting and logic check adjustments inuseAllowanceCheck
functionThe renaming of
erc20AllowanceData
toerc20AllowanceRes
and the type casting toCallResult
are appropriate given the context of handling the newstarknet.js
version. However, ensure that the propertyremaining
is correctly defined within theCallResult
type, as it is used to check the allowance status.context/StarknetIdJsProvider.tsx (1)
32-32
: Simplified Provider instantiation inStarknetIdJsProvider
The update to directly use
nodeUrl
in theProvider
constructor simplifies the code and presumably aligns with the new API structure ofstarknet.js
v6. This change should be tested to ensure that the provider is correctly instantiated and functional.utils/cacheDomainData.ts (1)
55-55
: Refactored data structure instoreDomainData
andgetDomainData
functionsChanging
existingData
from an array to a dictionary mapping enhances data access performance. Ensure that all interactions withexistingData
are updated to work with the new structure, including checks and updates.Also applies to: 66-66
hooks/useNeedAllowances.tsx (1)
70-74
: Updated balance calculation inuseNeedsAllowances
The conversion of
erc20AllowanceData
toerc20AllowanceRes
and the subsequent use offromUint256
for balance calculation align with the requirements for handling large number data types in blockchain contexts. Ensure that thefromUint256
function is correctly implemented and tested for accuracy.hooks/useNotificationManager.ts (1)
7-10
: Enhanced transaction receipt handling inuseNotificationManager
The introduction of specific types for transaction receipt responses and the detailed handling of each possible outcome are excellent improvements. This ensures that users are accurately informed about the status of their transactions. Verify that all new methods (
isRejected
,isReverted
,isError
,isSuccess
) are correctly implemented and that thematch
function properly handles each case.Also applies to: 31-58
types/frontTypes.d.ts (1)
133-133
: Approved type definition updates.The changes to the index signature syntax and the addition of the
CallResult
type are appropriate and improve type safety and clarity in the codebase.Also applies to: 179-181
components/identities/actions/clickable/clickablePersonhoodIcon.tsx (1)
16-16
: Correct and secure handling of type imports and usage.The update from
typedData
toTypedData
is correct, reflecting the changes in thestarknet.js
library. The usage ofTypedData
in thesignMessage
function call is also appropriate, ensuring type safety.Also applies to: 105-105
components/discount/freeRenewalDiscount.tsx (1)
288-289
: Use optional chaining for safer property access.The conditional access can be further improved by using optional chaining, which is safer and cleaner:
- needSubscription && needSubscription.needSubscription[domain]?.[currency] + needSubscription?.needSubscription[domain]?.[currency]Tools
Biome
[error] 288-289: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
Files selected for processing (1)
- package.json (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- package.json
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.
lgtm
I tested most tx, everything seems to work well but the bugs are still here
Summary by CodeRabbit
Dependency Updates
starknet
to version6.9.0
.starknetkit
to version^1.1.9
.Improvements
Refactors