fix: React props errors #4692
fix: React props errors #4692
Conversation
Branch preview✅ Deploy successful! Storybook: |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| <EthHashInfo address={option.label} name={option.name} shortAddress={false} copyAddress={false} /> | ||
| </Typography> | ||
| ) | ||
| }} | ||
| renderInput={(params) => ( | ||
| <AddressInput | ||
| data-testid="address-item" |
There was a problem hiding this comment.
The use of destructuring to extract key from props is unnecessary as key is not needed for the child components. Remove destructuring and pass props directly: return (<Typography {...props} data-testid="address-item" ...> This keeps the code concise and aligned with best practices.
| }: ExplorerButtonProps): ReactElement => { | ||
| }: ExplorerButtonProps): ReactElement | null => { | ||
| if (!href) return null | ||
|
|
||
| return isCompact ? ( | ||
| <Tooltip title={title} placement="top"> | ||
| <IconButton |
There was a problem hiding this comment.
Ensure href is included in the ExplorerButtonProps type definition. This prevents type errors and makes the purpose of the check clear. If href is not a required prop, consider if null is the appropriate fallback behavior or if a default action is preferable.
| </Typography> | ||
| } | ||
| /> | ||
| </NextLink> | ||
| ))} | ||
| </Tabs> | ||
| ) |
There was a problem hiding this comment.
Removing NextLinkComposed simplifies the code but using legacyBehavior is deprecated and should be avoided. Refactor using Link from mui to ensure compatibility and accessibility. Direct usage of NextLink in Tab removes focus management for keyboard users.
| <ChainIndicator chainId={chain.chainId} inline /> | ||
| </Box> | ||
| ) | ||
| }} | ||
| getOptionLabel={(option) => option.chainName} | ||
| getOptionDisabled={isOptionDisabled} | ||
| renderInput={(params) => ( |
There was a problem hiding this comment.
Consider extracting the renderOption function into a separate component to improve readability and reusability. Adopting this approach not only aligns with the DRY principle but also enhances code clarity and maintainability.
| {isRecommendedNonce && <NonceFormHeader>Recommended nonce</NonceFormHeader>} | ||
| {isInitialPreviousNonce && <NonceFormHeader sx={{ pt: 3 }}>Replace existing</NonceFormHeader>} | ||
| <NonceFormOption key={option} menuItemProps={props} nonce={option} /> | ||
| <NonceFormOption menuItemProps={rest} nonce={option} /> | ||
| </div> | ||
| ) | ||
| }} |
There was a problem hiding this comment.
Consider renaming key to something more descriptive like optionKey for clarity, given its significance in React's reconciliation. Check the safety of destructuring props without knowing its complete structure; ensure it doesn't remove required properties.
📦 Next.js Bundle Analysis for safe-wallet-webThis analysis was generated by the Next.js Bundle Analysis action. 🤖
|
| Page | Size (compressed) |
|---|---|
global |
1011.35 KB (🟡 +2 B) |
Details
The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.
Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis
If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!
Twenty Pages Changed Size
The following pages changed size from the code in this PR compared to its base branch:
| Page | Size (compressed) | First Load |
|---|---|---|
/apps |
29.65 KB (🟢 -79 B) |
1.02 MB |
/apps/custom |
27.71 KB (🟢 -79 B) |
1.01 MB |
/balances |
29.71 KB (🟢 -102 B) |
1.02 MB |
/balances/nfts |
9.55 KB (🟢 -92 B) |
1020.9 KB |
/new-safe/advanced-create |
26.45 KB (🟡 +14 B) |
1.01 MB |
/new-safe/create |
25.58 KB (🟡 +16 B) |
1.01 MB |
/new-safe/load |
6.08 KB (🟡 +23 B) |
1017.43 KB |
/settings/appearance |
2.24 KB (🟢 -90 B) |
1013.59 KB |
/settings/cookies |
1.86 KB (🟢 -93 B) |
1013.22 KB |
/settings/data |
1.79 KB (🟢 -88 B) |
1013.14 KB |
/settings/environment-variables |
3.27 KB (🟢 -89 B) |
1014.62 KB |
/settings/modules |
4.05 KB (🟢 -76 B) |
1015.4 KB |
/settings/notifications |
21.31 KB (🟢 -73 B) |
1.01 MB |
/settings/safe-apps |
14.22 KB (🟢 -87 B) |
1 MB |
/settings/security |
2.32 KB (🟢 -95 B) |
1013.67 KB |
/settings/setup |
30.73 KB (🟢 -82 B) |
1.02 MB |
/transactions |
93.08 KB (🟢 -72 B) |
1.08 MB |
/transactions/history |
93.04 KB (🟢 -72 B) |
1.08 MB |
/transactions/messages |
54.81 KB (🟢 -88 B) |
1.04 MB |
/transactions/queue |
43.9 KB (🟢 -100 B) |
1.03 MB |
Details
Only the gzipped size is provided here based on an expert tip.
First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.
Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis
Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this.
Coverage report
Show files with reduced coverage 🔻
Test suite run success1729 tests passing in 236 suites. Report generated by 🧪jest coverage report action from 70a9c29 |
| <button | ||
| aria-label="" | ||
| class="MuiButtonBase-root MuiIconButton-root MuiIconButton-sizeSmall css-1vbp2rr-MuiButtonBase-root-MuiIconButton-root" | ||
| data-mui-internal-clone-element="true" | ||
| data-testid="explorer-btn" | ||
| rel="noreferrer" | ||
| tabindex="0" | ||
| target="_blank" | ||
| type="button" | ||
| > | ||
| <mock-icon | ||
| aria-hidden="" | ||
| class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-tqxw8e-MuiSvgIcon-root" | ||
| focusable="false" | ||
| /> | ||
| </button> |
There was a problem hiding this comment.
This is not rendered anymore in the snapshot because we are not mocking the chain and with my change, the explorer link is not rendered anymore if there is no href
What it solves
Fixes several React errors in the console
How to test it
Checklist