Feat/react scan integration #8659
Conversation
- Add react-scan module with platform-specific implementations (web, desktop, extension, native) - Add ReactScanSetting component with controls for: - Enable/disable React Scan - Show/hide toolbar - Animation speed adjustment - Track unnecessary renders toggle - Integrate react-scan initialization in Bootstrap component - Auto-restart app when React Scan is enabled/disabled - Add react-scan dependencies to package.json
- Introduced a new ListItem for the Performance Monitor feature in the DevSettingsSection. - Added a Switch component to enable/disable the performance monitor, with functionality to update settings and restart the app upon change. - Removed the previous implementation of the Performance Monitor toggle for cleaner integration.
limichange
requested review from
ezailWang,
huhuanming,
originalix,
revan-zhang and
sidmorizon
as code owners
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
WalkthroughAdds React Scan support: dependency, persisted dev settings, settings UI, platform adapters (web/native/ext no-ops), a lazy-loaded module, a hook to initialize/configure it, and Bootstrap integration. Also memoizes one market list component. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App Bootstrap
participant Bootstrap as Bootstrap
participant Hook as useReactScan Hook
participant Atom as DevSettings Atom
participant Module as react-scan module
App->>Bootstrap: mount
Bootstrap->>Hook: call useReactScan()
Hook->>Module: initReactScan() %% init once (disabled)
Module-->>Hook: resolved
Note over Atom,Hook: reads persisted dev settings
Hook->>Atom: read enableReactScan, showToolbar, animationSpeed, trackUnnecessaryRenders
alt enableReactScan true
Hook->>Module: scanAsync({enabled:true, showToolbar, animationSpeed, trackUnnecessaryRenders})
Module->>Module: lazy-import react-scan & apply config
else enableReactScan false
Hook->>Module: scanAsync({enabled:false})
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🔇 Additional comments (2)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
apps/desktop/index.js(1 hunks)apps/web/index.js(1 hunks)package.json(1 hunks)packages/kit-bg/src/states/jotai/atoms/devSettings.ts(2 hunks)packages/kit/src/provider/Bootstrap.tsx(3 hunks)packages/kit/src/views/Market/MarketHomeV2/components/MarketTokenList/MarketTokenListBase.tsx(2 hunks)packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/ReactScanSetting.tsx(1 hunks)packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/index.tsx(2 hunks)packages/shared/src/modules3rdParty/react-scan/index.ext.ts(1 hunks)packages/shared/src/modules3rdParty/react-scan/index.native.ts(1 hunks)packages/shared/src/modules3rdParty/react-scan/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unittest (20.x)
- GitHub Check: lint (20.x)
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
apps/desktop/index.js (1)
3-6: LGTM! Clean startup integration.The initialization is placed correctly before
registerRootComponent, and the module handles platform stubs internally.apps/web/index.js (1)
10-14: LGTM! Consistent with desktop startup.Initialization follows the pattern: Sentry → ReactScan → Intercom. Clean and consistent.
packages/shared/src/modules3rdParty/react-scan/index.ext.ts (1)
1-8: LGTM! Clean platform stub.No-op exports for extension platform prevent import errors. Standard pattern.
packages/shared/src/modules3rdParty/react-scan/index.native.ts (1)
1-8: LGTM! Native platform stub.Mirrors the extension stub pattern. Clean and consistent.
packages/shared/src/modules3rdParty/react-scan/index.ts (1)
1-9: LGTM! Clean initialization pattern.Re-exports
scanand providesinitReactScan()that starts disabled. Bootstrap will enable it conditionally via dev settings.packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/index.tsx (1)
575-594: LGTM! Performance Monitor toggle looks correct.Switch updates dev settings and restarts app. Logic is clean.
packages/kit/src/provider/Bootstrap.tsx (1)
655-655: LGTM! Hook invocation is clean.Called in Bootstrap at the right spot, after other initialization hooks.
packages/kit-bg/src/states/jotai/atoms/devSettings.ts (2)
55-59: Type definitions look good.The new React Scan settings are:
- All optional (maintains backward compatibility)
animationSpeeduses a string union type (type-safe)- Boolean flags are explicitly typed
No issues found.
94-97: Default values are consistent and sensible.The defaults align with typical React Scan usage:
- Disabled by default (safe choice for production-like dev env)
- Toolbar shown when enabled (helps developers discover features)
- Fast animation (good balance of visibility and performance)
- Unnecessary renders tracked (the main value of React Scan)
No issues found.
packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/ReactScanSetting.tsx (1)
54-57: Handle restartApp() errors
TherestartApp()promise is ignored in ReactScanSetting.tsx (lines 54-57). Confirm ifserviceApp.restartApp()handles failures internally; if not, add.catch()to log errors and notify the user.packages/kit/src/views/Market/MarketHomeV2/components/MarketTokenList/MarketTokenListBase.tsx (1)
1-1: LGTM: Import added for memoization.The
memoimport is correctly added to support the memoization below.
packages/kit/src/views/Market/MarketHomeV2/components/MarketTokenList/MarketTokenListBase.tsx
Show resolved
Hide resolved
packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/ReactScanSetting.tsx
Show resolved
Hide resolved
packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/ReactScanSetting.tsx
Show resolved
Hide resolved
packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/ReactScanSetting.tsx
Show resolved
Hide resolved
packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/ReactScanSetting.tsx
Show resolved
Hide resolved
packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/ReactScanSetting.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
apps/desktop/index.js(1 hunks)apps/web/index.js(1 hunks)package.json(1 hunks)packages/kit-bg/src/states/jotai/atoms/devSettings.ts(2 hunks)packages/kit/src/provider/Bootstrap.tsx(3 hunks)packages/kit/src/views/Market/MarketHomeV2/components/MarketTokenList/MarketTokenListBase.tsx(2 hunks)packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/ReactScanSetting.tsx(1 hunks)packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/index.tsx(2 hunks)packages/shared/src/modules3rdParty/react-scan/index.ext.ts(1 hunks)packages/shared/src/modules3rdParty/react-scan/index.native.ts(1 hunks)packages/shared/src/modules3rdParty/react-scan/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint (20.x)
- GitHub Check: unittest (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (13)
packages/shared/src/modules3rdParty/react-scan/index.ext.ts (1)
1-8: LGTM!Clean no-op stubs for the extension platform. The comments clearly indicate why these are no-ops.
packages/shared/src/modules3rdParty/react-scan/index.native.ts (1)
1-8: LGTM!Clean no-op stubs for the native platform. Matches the extension variant appropriately.
packages/kit-bg/src/states/jotai/atoms/devSettings.ts (2)
55-59: LGTM!React Scan settings are well-structured with appropriate types. The union type for animation speed is clean.
94-97: LGTM!Default values are sensible: disabled by default to avoid surprises, but configured for optimal experience when enabled.
packages/kit/src/views/Market/MarketHomeV2/components/MarketTokenList/MarketTokenListBase.tsx (2)
1-1: LGTM!Importing
memofor performance optimization.
261-265: Ensure stable props for memoized componentIn MarketNormalTokenList.tsx and MarketWatchlistTokenList.tsx, onItemPress and toolbar are passed directly (no useCallback/useMemo), so the memo wrapper won’t skip re-renders. Memoize these callbacks and object props upstream or add a custom comparator to
memo.packages/shared/src/modules3rdParty/react-scan/index.ts (1)
1-9: LGTM!Clean implementation. Initializes React Scan in disabled state, letting Bootstrap enable it based on dev settings. The re-export of
scanprovides flexibility for direct usage.apps/desktop/index.js (2)
3-3: LGTM!Importing the React Scan initialization function.
6-7: LGTM!Initializing React Scan early, before the root component registers. This ensures the dev tool is ready when React starts rendering.
apps/web/index.js (2)
10-10: LGTM!Importing React Scan initialization.
13-14: LGTM!Initializing React Scan after Sentry is a sensible order—error tracking is ready first, then the dev tool.
packages/kit/src/provider/Bootstrap.tsx (1)
576-600: LGTM! Clean integration of React Scan in Bootstrap.The hook correctly:
- Guards execution to web/desktop dev environments
- Reads settings with proper fallback defaults
- Updates scan configuration reactively
- Includes complete dependencies
packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/index.tsx (1)
73-73: LGTM! ReactScanSetting properly integrated.The component is imported and rendered in the appropriate location within DevSettingsSection.
Also applies to: 596-596
packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/ReactScanSetting.tsx
Show resolved
Hide resolved
packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/ReactScanSetting.tsx
Show resolved
Hide resolved
packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/ReactScanSetting.tsx
Show resolved
Hide resolved
- Moved the React Scan logic from Bootstrap to a dedicated hook (useReactScan) for better separation of concerns. - Updated the scan and initReactScan functions to be asynchronous, allowing for improved handling of initialization and settings application. - Enhanced the useReactScan hook to manage React Scan settings dynamically based on development settings. - Cleaned up imports and removed deprecated code related to the previous implementation.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/kit/src/provider/hooks/useReactScan.ts (1)
18-46: Verify repeatedscanAsynccalls don't cause issues.The effect calls
scanAsyncon every settings change. Past review comments suggestscan()should be called once at startup, withsetOptions()used for runtime changes. The current approach callsinitReactScan()(which callsscanAsyncinternally) on first mount, then callsscanAsyncagain whenever settings change.Confirm the react-scan library handles this pattern correctly, or refactor to use
setOptions()for runtime updates.react-scan setOptions API for runtime configuration changes
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
packages/kit/src/provider/Bootstrap.tsx(2 hunks)packages/kit/src/provider/hooks/useReactScan.ts(1 hunks)packages/shared/src/modules3rdParty/react-scan/index.ext.ts(1 hunks)packages/shared/src/modules3rdParty/react-scan/index.native.ts(1 hunks)packages/shared/src/modules3rdParty/react-scan/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (7)
packages/shared/src/modules3rdParty/react-scan/index.ext.ts (1)
1-8: LGTM! Clean platform stubs.The no-op implementations correctly provide compatibility for the extension platform where react-scan isn't supported.
packages/shared/src/modules3rdParty/react-scan/index.ts (2)
12-14: LGTM! Clean initialization wrapper.The function correctly initializes react-scan in a disabled state, following the expected startup pattern.
2-10: Lazy-loading pattern is correct.scan()usage verified as proper.The dynamic import reduces bundle size effectively. React-scan's
scan()API is imperative and void—calling it without await is correct. The code handles the promise chain properly through the async wrapper.packages/shared/src/modules3rdParty/react-scan/index.native.ts (1)
1-8: LGTM! Consistent platform stubs.The no-op implementations match the extension file pattern and correctly handle the native platform where react-scan isn't supported.
packages/kit/src/provider/hooks/useReactScan.ts (1)
12-14: LGTM! Correct platform gating.The conditional export ensures the hook only runs on web/desktop in development mode, with a noop fallback for other platforms.
packages/kit/src/provider/Bootstrap.tsx (2)
58-59: LGTM! Clean import.The import statement is properly placed with other hook imports.
629-629: LGTM! Proper bootstrap integration.The hook is correctly invoked during the Bootstrap phase, ensuring React Scan initializes at app startup.
… points - Eliminated the initReactScan function calls from both desktop and web index files to streamline the application startup process. - Cleaned up imports related to React Scan, enhancing code clarity and maintainability.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
package.json (1)
149-149: react-scan should be in devDependencies.This dev-only tool was already flagged in past reviews—move it to devDependencies to keep production bundles lean.
packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/index.tsx (1)
620-620: AutoUpdateSettings must be imported.This component is undefined per static analysis. Import it to prevent a runtime crash.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
package.json(1 hunks)packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/index.tsx(2 hunks)
🧰 Additional context used
🪛 ESLint
packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/index.tsx
[error] 620-620: 'AutoUpdateSettings' is not defined.
(react/jsx-no-undef)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unittest (20.x)
- GitHub Check: lint (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/kit/src/views/Setting/pages/Tab/DevSettingsSection/index.tsx (3)
609-609: Consider moving DebugRenderTracker below ReactScanSetting.A reviewer (sidmorizon) suggested placing the DebugRenderTracker config after
<ReactScanSetting />. The block is currently at lines 566–586.Based on learnings
611-618: showPerpsRenderStats section looks good.Clean implementation with appropriate Switch control.
588-607: Performance Monitor duplication verified—resolved.The old block at lines 574–580 has been removed. Only one Performance Monitor toggle remains in the file (line 590). No duplication.
- Eliminated the AutoUpdateSettings component from the DevSettingsSection to streamline the settings interface and improve code clarity.
- Added a new useReactScan hook to manage React Scan initialization and settings dynamically based on development settings. - Removed the previous implementation of useReactScan from the provider hooks to streamline the code structure. - Updated Bootstrap to utilize the new useReactScan hook, enhancing separation of concerns and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/kit/src/hooks/useReactScan.ts (1)
47-47: Add error handling for async operations.Errors from
initReactScan()orscanAsync()are unhandled. Attach a.catch()block to log failures.Apply this fix:
- void setupReactScan(); + void setupReactScan().catch((error) => { + console.error('React Scan setup failed:', error); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/kit/src/hooks/useReactScan.ts(1 hunks)packages/kit/src/provider/Bootstrap.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: lint (20.x)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: unittest (20.x)
🔇 Additional comments (3)
packages/kit/src/provider/Bootstrap.tsx (1)
49-49: LGTM! Clean integration.The import and hook call follow the existing Bootstrap pattern. Placement is consistent with other lifecycle hooks.
Also applies to: 628-628
packages/kit/src/hooks/useReactScan.ts (2)
12-13: Good platform gating.The conditional export prevents bundling react-scan into native builds. Clean approach.
48-53: Dependencies are correct.All relevant settings fields are included. Optional chaining handles undefined settings properly.
Summary by CodeRabbit
New Features
Chores