fix: fix useApi interaction with useApiWithChain2 #1556
Conversation
WalkthroughThe changes introduce two new functions, Changes
Assessment against linked issues
Recent review detailsConfiguration used: CodeRabbit UI 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/extension-polkagate/src/hooks/useApi.ts (2 hunks)
Additional context used
Biome
packages/extension-polkagate/src/hooks/useApi.ts
[error] 207-207: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
packages/extension-polkagate/src/hooks/useApi.ts (2)
106-107: Consider removing the commented-out console.log statementCommented-out code can clutter the codebase and make it harder to maintain. If this log statement is no longer needed, it's best to remove it entirely. If it's kept for debugging purposes, consider using a logging library that allows for different log levels, so debug logs can be easily toggled on/off.
249-252: Consider removing the commented-out console.log statementSimilar to the previous comment, it's generally best to remove commented-out code to keep the codebase clean. If this log statement is needed for debugging, consider using a logging library with configurable log levels.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/extension-polkagate/src/hooks/useApi.ts (5 hunks)
Additional comments not posted (2)
packages/extension-polkagate/src/hooks/useApi.ts (2)
198-217: Well-implementedhandleManualApiRequestfunctionThe
handleManualApiRequestfunction is well-structured and follows good practices:
- It checks for existing API connections before initiating a new one, avoiding unnecessary reconnections.
- The use of optional chaining (
savedApi?.api?.isConnected) is a good practice for safe property access.- The function handles different scenarios (already requested, connected, new connection) appropriately.
Good job on this implementation!
221-226: Good addition of manual API request handling inuseEffectThe addition of checks for
_endpointand_genesisHashat the beginning of theuseEffecthook is a good improvement. It allows for more flexible API connection management, especially when integrating withuseApiWithChain2. The early return when these conditions are met helps to avoid unnecessary processing.
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (2)
packages/extension-polkagate/src/hooks/useApi.ts (2)
106-107: Consider removing the commented-out console.log statementCommented-out code can clutter the codebase and make it harder to maintain. If this log statement is no longer needed, it's best to remove it entirely. If it's kept for debugging purposes, consider using a logging library that allows for different log levels, so debug logs can be easily toggled on/off.
249-252: Remove commented-out console.log statementCommented-out code can clutter the codebase and make it harder to maintain. If this log statement is no longer needed, it's best to remove it entirely. If it's kept for debugging purposes, consider using a logging library that allows for different log levels, so debug logs can be easily toggled on/off.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/extension-polkagate/src/hooks/useApi.ts (5 hunks)
Additional comments not posted (2)
packages/extension-polkagate/src/hooks/useApi.ts (2)
Line range hint
127-134: Avoid direct state mutation inhandleNewApifunctionThe current implementation directly mutates the
apisContext.apisobject, which can lead to unexpected behavior in React applications. It's recommended to create new objects to maintain immutability.Apply this diff to avoid direct state mutation:
- apisContext.apis[genesisHash] = toSaveApi; - apisContext.setIt({ ...apisContext.apis }); + apisContext.setIt((prevApis) => ({ + ...prevApis, + [genesisHash]: toSaveApi + }));This approach updates the state immutably using the setter function of
apisContext.setIt.
189-196: Avoid direct state mutation inaddApiRequestfunctionThe current implementation directly mutates the
apisContext.apisobject. To maintain immutability and prevent unexpected behavior, consider using an immutable update pattern.Apply this diff to avoid direct state mutation:
const addApiRequest = useCallback((endpointToRequest: string, genesisHash: string) => { - const toSaveApi = apisContext.apis[genesisHash] ?? []; + const toSaveApi = [...(apisContext.apis[genesisHash] ?? [])]; toSaveApi.push({ endpoint: endpointToRequest, isRequested: true }); - apisContext.apis[genesisHash] = toSaveApi; - apisContext.setIt({ ...apisContext.apis }); + apisContext.setIt((prevApis) => ({ + ...prevApis, + [genesisHash]: toSaveApi + })); }, [apisContext]);This approach creates a new array for
toSaveApiand uses the setter function ofapisContext.setItto update the state immutably.
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (2)
packages/extension-polkagate/src/hooks/useApi.ts (2)
106-106: Consider using a logging library for better debugging controlWhile commenting out
console.logstatements is good for production, it might hinder debugging. Consider using a logging library or environment-based logging to maintain debugging capabilities without affecting production code.You could implement a simple logging utility:
const log = (message: string, ...args: any[]) => { if (process.env.NODE_ENV !== 'production') { console.log(message, ...args); } }; // Usage log('Successfully connected to existing API for genesis hash:', genesisHash);
249-251: Consider using a logging library for better debugging controlWhile commenting out
console.logstatements is good for production, it might hinder debugging. Consider using a logging library or environment-based logging to maintain debugging capabilities without affecting production code.You could implement a simple logging utility as suggested in a previous comment.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/extension-polkagate/src/hooks/useApi.ts (5 hunks)
Additional comments not posted (2)
packages/extension-polkagate/src/hooks/useApi.ts (2)
Line range hint
127-134: LGTM: Proper handling of new API entryThe logic for adding a new API entry to
toSaveApiis correct. SettingisRequestedto false is appropriate as the API is now connected.
219-224: LGTM: New useEffect hook for handling _endpoint and _genesisHashThe new
useEffecthook correctly handles the case when_endpointand_genesisHashare available, which is crucial for the interaction withuseApiWithChain2.
## [0.12.3](v0.12.2...v0.12.3) (2024-09-23) ### Bug Fixes * fix useApi interaction with useApiWithChain2 ([#1556](#1556)) ([23f4b36](23f4b36))
Works Done
Close: #1555
Summary by CodeRabbit
New Features
Bug Fixes