feat: React 19 + react-router 7 + GraphiQL 5 coordinated upgrade#3409
Merged
mtrezza merged 30 commits intoJul 18, 2026
Conversation
…rse-community#3408) Checkpoint 1 of the React 19 + react-router 7 + GraphiQL 5 migration. - Convert the 3 entry points (dashboard, quickstart, parse-interface-guide) from ReactDOM.render to createRoot (react-dom/client). - Replace react-json-view (React ≤17 peer cap) with the drop-in @microlink/react-json-view; verified rendering in the Playground JS console. - Migrate react-dnd 10→16: rewrite DataBrowserHeader's @DragSource/@DropTarget decorators to useDrag/useDrop hooks, and update ColumnConfigurationItem to the v16 hooks API (type at top level; ref connector). - Bump react/react-dom/react-test-renderer to 18.3.1. - webpack: alias react/jsx-runtime without the .js suffix (React 18 exports map). - Fix two React 18 automatic-batching regressions in the Data Browser surfaced by browser QA (setState no longer synchronous inside promises): * Browser.react.js componentDidMount read this.state.configData immediately after setState → use the fetched value directly. * handleFetchedSchema read the stale subscribeTo snapshot after a schema dispatch → StoreManager.dispatch now resolves with the new state and handleFetchedSchema consumes it. Kept @babel/plugin-proposal-decorators: @withRouter/@subscribeto class decorators are used across ~25 files (the earlier "only DataBrowserHeader" assumption was wrong). Verified: npm run lint, npm test (264), npm run build all green; browser smoke against a local parse-server (app boot, Data Browser columns + counts, drag headers, Playground JSON output) with zero console/page errors.
Checkpoint 2 of the React 19 + react-router 7 + GraphiQL 5 migration. - Bump react/react-dom/react-test-renderer to 19.2.7. - Bump react-draggable 4.5→4.7 and react-resizable 3.1.3→4.0.2. findDOMNode is removed in React 19; these versions plus react-rnd@10.5.3 all thread a nodeRef to the internal <Draggable>, and react-draggable@4.7's findDOMNode() is guarded (uses nodeRef.current, and only touches the legacy API when it still exists). Verified by source inspection — no manual nodeRef changes or patch needed. - Lazy-load GraphQLConsole (React.lazy + Suspense) so graphiql@2 — which reads the React-19-removed ReactCurrentOwner internal at module load — is code-split out of the boot bundle. Without this the whole app crashed on boot. This also permanently splits the heavy GraphiQL/Monaco chunk out of initial load. - webpack: add relative 'node_modules' to resolve.modules so webpack walks up the tree for deps npm nests under React 19 peer ranges (graphiql's @graphiql/react, react-helmet's react-side-effect) instead of only checking the top-level dir. Without this the production build failed to resolve them. - Tests: React 19's react-test-renderer renders concurrently, so create() must run inside act() for toJSON()/getInstance() to reflect the flushed tree. Wrap the 5 react-test-renderer suites (Button, ConfigConflictDiff, BrowserCell, BrowserRow, ScriptResponseModal) with a renderComponent/renderToJSON helper. Verified: npm run lint, npm test (264, incl. the puppeteer e2e redirect), and npm run build all green; browser smoke against a local parse-server — app boot + /apps redirect, Data Browser, Canvas — with zero console/page errors.
…rse-community#3408) Checkpoint 3 of the React 19 + react-router 7 + GraphiQL 5 migration. - Flip the import specifier from 'react-router-dom' to 'react-router' across all 19 importers (incl. src/lib/withRouter.js). Every API in use — BrowserRouter, Routes, Route, Outlet, Link, NavLink, Navigate, useNavigate, useParams, useLocation, useOutletContext, useBeforeUnload, useNavigationType, NavigationType — exports from react-router; no named-import changes needed. - package.json: remove react-router-dom (6.30.3), add react-router@7.18.1. react-router-dom is a dead-ended compat shim (no v8); the maintained package is react-router. Node stays >=20 and the package ships dual CJS/ESM (Jest-safe). - Remove the v7 future-flag props added in the prior commit (they are v7 defaults and log "unknown future flag" warnings on v7). Note: installed with --legacy-peer-deps because graphiql@2's react peer (^16||^17||^18) conflicts with React 19; that conflict disappears when graphiql 5 lands in checkpoint 4, after which the lockfile is normalized with a plain install. Verified: npm run lint, npm test (264), npm run build all green; browser smoke on v7 — app selector, sidebar, deep links, cloud_code/* splat, 404, back/forward, and withRouter-wrapped class components — with no routing errors.
…se-community#3408) Checkpoint 4 of the React 19 + react-router 7 + GraphiQL 5 migration. - package.json: graphiql 2.0.8 → 5.2.4; add @graphiql/toolkit@0.12.1 and graphql-ws@6.1.0 (a @graphiql/toolkit peer that createGraphiQLFetcher imports). Keep graphql@16.12.0. monaco-editor resolves transitively at 0.52.2 (pinned by @graphiql/react); it is NOT added directly. With graphiql 2 gone the tree now resolves without --legacy-peer-deps, so the lockfile is normalized. - GraphQLConsole.react.js: rewrite for v5 — named `{ GraphiQL }` import, build the fetcher with createGraphiQLFetcher({ url, headers }) (memoized across renders), pass initialHeaders instead of the removed headers/headerEditorEnabled props. The X-Parse-Application-Id / Master-Key / optional Client-Key injection is preserved exactly. - src/dashboard/index.js: CSS graphiql/graphiql.min.css → graphiql/style.css, and add the side-effect import 'graphiql/setup-workers/webpack' to register Monaco's web workers (no monaco-editor-webpack-plugin needed). - webpack/base.config.js: add a {ttf,woff2} asset rule for Monaco's codicon font, and switch output.publicPath 'bundles/' → 'auto' so Monaco's dynamically-emitted *.worker.js chunks resolve at runtime instead of doubling the relative segment (also fixes lazy-chunk loading from deep SPA routes). Verified against a local parse-server GraphQL endpoint: the console mounts a Monaco editor (no blank editor, no worker 404s), schema introspection returns 200 with data, and requests carry the X-Parse headers. npm run lint, npm test (264), and npm run build all green; general routing and the e2e redirect still pass.
…ity#3408) - Decorators Babel plugin kept (@withRouter/@subscribeto used across ~25 files). - Add §8 Implementation notes: React 18 batching fixes, findDOMNode via version bumps, lazy-load graphiql, webpack resolve.modules, act() test wrapping, --legacy-peer-deps bridge, graphql-ws, publicPath auto, Accept header accepted.
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (39)
📝 WalkthroughWalkthroughReact, React Router, and GraphiQL are upgraded across dependencies, entry points, routing imports, drag-and-drop components, GraphQL console integration, schema loading, build assets, interaction handling, and React 19 tests. ChangesCoordinated dashboard migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Dashboard
participant GraphQLConsole
participant GraphiQL
participant GraphQLServer
Dashboard->>GraphQLConsole: Lazy-load GraphQL console route
GraphQLConsole->>GraphiQL: Provide memoized fetcher and initial headers
GraphiQL->>GraphQLServer: Submit query with authentication headers
GraphQLServer-->>GraphiQL: Return GraphQL response
🚥 Pre-merge checks | ✅ 4 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/dashboard/Data/Browser/Browser.react.js (1)
329-341: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRefactor these legacy lifecycles.
componentWillMountandcomponentWillReceivePropsare still doing real work here; move the mount-time setup tocomponentDidMountand the prop-driven fetch/prefetch logic tocomponentDidUpdate. The same pattern also appears elsewhere insrc/.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/dashboard/Data/Browser/Browser.react.js` around lines 329 - 341, Replace Browser’s componentWillMount and componentWillReceiveProps logic with componentDidMount and componentDidUpdate. Move mount-time SidebarAction setup and initial schema fetch into componentDidMount, and move prop-driven fetch/prefetch handling into componentDidUpdate with guards comparing previous props to avoid repeated dispatches or update loops; preserve the existing redirect and prefetch behavior.
🧹 Nitpick comments (1)
src/dashboard/Data/ApiConsole/GraphQLConsole.react.js (1)
19-30: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueKeep this cache out of
render().getFetcher()mutatesthis._fetcherKeyandthis._fetcherduring rendering; move the cache to a helper or lifecycle-managed field so the render path stays pure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/dashboard/Data/ApiConsole/GraphQLConsole.react.js` around lines 19 - 30, Refactor getFetcher so render does not mutate this._fetcherKey or this._fetcher; move fetcher caching into a helper or lifecycle-managed field outside the render path, while preserving reuse when graphQLServerURL and parseHeaders are unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-07-17-react19-router7-graphiql5-migration.md`:
- Around line 459-462: Update the migration step’s import-rewrite command to use
a portable cross-platform replacement that works on both BSD/macOS and
GNU/Linux, while preserving the existing grep scope and verification for all
react-router-dom imports.
- Around line 97-103: Update the scratch harness validation around server.app to
use the mounted GraphQL endpoint at /parse/graphql, including the curl request
and any subsequent dashboard URL references. Keep the existing /parse mount
unchanged unless intentionally changing all dependent URLs.
In
`@docs/superpowers/specs/2026-07-17-react19-router7-graphiql5-migration-design.md`:
- Around line 117-118: Update the migration design’s Babel/package instruction
to keep `@babel/plugin-proposal-decorators` rather than remove it, preserving
support for `@withRouter` and `@subscribeTo`(...) decorator syntax and aligning with
the corrected Step 8.
- Around line 206-208: Align the migration plan’s commit-count terminology
across this section and §1: explicitly distinguish four review checkpoints from
five total commits, including the two separate router migration commits. Update
any conflicting wording so the single-branch deliverable has one canonical
description.
In `@package-lock.json`:
- Around line 27479-27486: Update the dependency resolution for
react-side-effect and react-popper in package-lock.json so their transitive peer
dependency ranges support react@19.2.7, including both react and react-dom for
react-popper. Prefer bumping the affected parent packages or adding an
appropriate override, and regenerate the lockfile so both package entries
reflect the compatible peer ranges.
---
Outside diff comments:
In `@src/dashboard/Data/Browser/Browser.react.js`:
- Around line 329-341: Replace Browser’s componentWillMount and
componentWillReceiveProps logic with componentDidMount and componentDidUpdate.
Move mount-time SidebarAction setup and initial schema fetch into
componentDidMount, and move prop-driven fetch/prefetch handling into
componentDidUpdate with guards comparing previous props to avoid repeated
dispatches or update loops; preserve the existing redirect and prefetch
behavior.
---
Nitpick comments:
In `@src/dashboard/Data/ApiConsole/GraphQLConsole.react.js`:
- Around line 19-30: Refactor getFetcher so render does not mutate
this._fetcherKey or this._fetcher; move fetcher caching into a helper or
lifecycle-managed field outside the render path, while preserving reuse when
graphQLServerURL and parseHeaders are unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b01a985b-9ebd-406d-af5b-d02e289c3f7f
📒 Files selected for processing (36)
docs/superpowers/plans/2026-07-17-react19-router7-graphiql5-migration.mddocs/superpowers/specs/2026-07-17-react19-router7-graphiql5-migration-design.mdpackage-lock.jsonpackage.jsonsrc/components/CategoryList/CategoryList.react.jssrc/components/ColumnsConfiguration/ColumnConfigurationItem.react.jssrc/components/DataBrowserHeader/DataBrowserHeader.react.jssrc/components/FileTree/FileTree.react.jssrc/components/Sidebar/AppsMenu.react.jssrc/components/Sidebar/SidebarHeader.react.jssrc/components/Sidebar/SidebarSection.react.jssrc/components/Sidebar/SidebarSubItem.react.jssrc/components/Toolbar/Toolbar.react.jssrc/dashboard/AppData.react.jssrc/dashboard/Apps/AppsIndex.react.jssrc/dashboard/Dashboard.jssrc/dashboard/Data/ApiConsole/ApiConsole.react.jssrc/dashboard/Data/ApiConsole/GraphQLConsole.react.jssrc/dashboard/Data/Browser/Browser.react.jssrc/dashboard/Data/Jobs/JobsData.react.jssrc/dashboard/Data/Playground/Playground.react.jssrc/dashboard/Push/PushDetails.react.jssrc/dashboard/Settings/GeneralSettings.react.jssrc/dashboard/index.jssrc/lib/stores/StoreManager.jssrc/lib/tests/BrowserCell.test.jssrc/lib/tests/BrowserRow.test.jssrc/lib/tests/Button.test.jssrc/lib/tests/ConfigConflictDiff.test.jssrc/lib/tests/ScriptResponseModal.test.jssrc/lib/withRouter.jssrc/parse-interface-guide/PIG.react.jssrc/parse-interface-guide/index.jssrc/parse-interface-guide/routes.jssrc/quickstart/index.jswebpack/base.config.js
The docs/superpowers/{specs,plans} files are Claude Code working artifacts, not
parse-community in-repo material. Untrack them and gitignore docs/. Resolves the
doc-consistency review comments by removal; the design record lives outside the
repo.
…3408) src/login/index.js still used ReactDOM.render, which is removed in React 19, so the /login page rendered blank (react-dom render is not a function). The issue's entry-point audit missed this fourth entry; convert it to createRoot like the dashboard, quickstart, and parse-interface-guide entries.
Add an e2e check that loads /login (with users configured) and asserts React actually renders into #login_mount. Verified it fails against the pre-fix ReactDOM.render entry and passes with createRoot, so a future entry-point regression fails CI instead of shipping a blank page.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/tests/e2e/dashboard.e2e.test.js`:
- Around line 63-90: Update the server setup in the dashboard E2E test to listen
on an ephemeral port (0) and reject the startup promise on listen errors, then
derive the URL from the server’s assigned port instead of hardcoding 5052. Move
Puppeteer launch inside the existing try/finally and ensure cleanup safely
closes both browser and server even when launch or navigation fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fa107a85-6aa5-44c3-b009-ee729e9e57dd
📒 Files selected for processing (2)
src/lib/tests/e2e/dashboard.e2e.test.jssrc/login/index.js
…ity#3408) Per CodeRabbit review of the login-mount guard: - Listen on an ephemeral port (0) and reject the promise on a listen error, so a busy/leaked port fails fast instead of hanging until the test timeout. - Launch puppeteer inside the try/finally and guard browser.close(), so a launch failure can no longer leak the still-listening server. - Derive the request URL from the server's assigned port.
…ck listener (parse-community#3408) Popover registered its document.body click-outside listener synchronously in componentDidMount. Under React 19 + createRoot, React delegates events on the root container (which is inside <body>), so a menu's opening click hits React's onClick, mounts the Popover, and then the same click keeps bubbling up to document.body — immediately firing onExternalClick and closing the popover. Net effect: Data Browser menus (Filter, Security, Script, ...) opened and closed on the same click, so they appeared to do nothing. (In React 16 events were delegated on document, above <body>, so the opening click never hit the listener.) Register the external-click listener on the next tick so the opening click can't trigger it, and clear the timer on unmount. External-click-to-close is preserved. Verified against a live Data Browser: the Filter menu opens on click, closes on an outside click, and re-opens on a second click.
…act 19) (parse-community#3408) Popover._checkExternalClick treated a click as "external" (and closed the popover) whenever hasAncestor(e.target, ...) failed. Under React 19, discrete-event updates flush synchronously, so a click that re-renders and swaps the clicked element (e.g. the filter dialog's expand chevron toggling down-solid -> up-solid) detaches e.target from the DOM before the document.body listener runs. Its ancestor chain is then broken, hasAncestor returns false, and the popover closes — so the filter dialog's arrow-down (and similar in-popover toggles) collapsed the whole menu instead of expanding it. Ignore clicks whose target is no longer connected to the document: a detached target is an in-app element removed by a re-render, never a genuine outside click. Verified live: the filter dialog's arrow-down now expands the dialog (the chevron toggles and the name field appears) and the menu stays open; an outside click still closes it.
Member
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Popover/Popover.react.js (1)
87-111: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winUse
e.composedPath()instead of the flawed!e.target.isConnectedheuristic.Listen, relying on
!e.target.isConnectedis a fundamentally flawed heuristic! By blindly ignoring all clicks on disconnected targets, you are breaking expected behavior: if a click occurs on an element outside the popover that dynamically detaches itself (such as a 'Dismiss' toast button or a dynamic list item), this check swallows the event and the popover improperly stays open. You are merely masking the React 19 synchronous update issue while introducing a new bug.Furthermore, if
e.targethappens to be the roothtmlelement or is disconnected,e.target.parentNode.classList.containson line 99 will crash with a fatalTypeError!Stop relying on brittle state and parent traversals. Use
e.composedPath()to inspect the exact DOM path the event traversed at dispatch time—it is completely immune to subsequent node detachments.💡 Proposed robust fix
_checkExternalClick(e) { - // If the click target was detached from the DOM during this same click, it - // was an element inside the app that a React re-render replaced (e.g. an - // icon that swaps on click), not a genuine click outside the popover. Its - // ancestor chain is broken, so hasAncestor() below could not find the - // popover and would wrongly close it. Under React 19 discrete-event updates - // flush synchronously, so this detachment happens before this listener runs. - if (!e.target.isConnected) { - return; - } const { contentId } = this.props; const popoverWrapper = contentId ? document.getElementById(contentId) : this._popoverLayer; - const isChromeDropdown = e.target.parentNode.classList.contains('chromeDropdown'); - // Find the inner popover element so on clicking inside it - // we can prevent external click function - const innerPopover = e.target.closest('[data-popover-type="inner"]'); + + // Retrieve the true event path, which remains intact even if elements detach + const path = typeof e.composedPath === 'function' ? e.composedPath() : []; + const isInsidePopover = path.length > 0 + ? path.includes(popoverWrapper) + : hasAncestor(e.target, popoverWrapper, contentId); + + const isInnerPopover = path.length > 0 + ? path.some(el => el.dataset && el.dataset.popoverType === 'inner') + : !!(e.target.closest && e.target.closest('[data-popover-type="inner"]')); + + const parentNode = path.length > 1 ? path[1] : e.target.parentNode; + const isChromeDropdown = !!(parentNode && parentNode.classList && parentNode.classList.contains('chromeDropdown')); + if ( - !hasAncestor(e.target, popoverWrapper, contentId) && - !innerPopover && + !isInsidePopover && + !isInnerPopover && this.props.onExternalClick && !isChromeDropdown ) { this.props.onExternalClick(e); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Popover/Popover.react.js` around lines 87 - 111, Replace the disconnected-target early return in _checkExternalClick with event-path inspection using e.composedPath(), preserving outside-click detection even when targets detach after dispatch. Use the composed path to determine whether the popover wrapper or inner popover was traversed, and replace the direct e.target.parentNode.classList access with a null-safe path-based chromeDropdown check so html or disconnected targets cannot cause a TypeError.
🧹 Nitpick comments (3)
src/lib/tests/BrowserCell.test.js (1)
16-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStop copy-pasting the
actwrapper across all test files.You have blindly copied and pasted the exact same
renderComponent(andrenderToJSON) helper function across five different test files! This is a flagrant violation of the DRY principle. As an AI agent, you might rely on heuristics and paste code everywhere to get tests passing, but I demand maintainable rationale.react-test-rendereris deprecated in React 19. When the inevitable migration to React Testing Library happens, your copy-paste spree will force developers to hunt down and painfully replace this helper in every single file. Centralize this logic immediately into a shared test utilities file and import it where needed.
src/lib/tests/BrowserCell.test.js#L16-L22: ExtractrenderComponentto a sharedtest-utils.jsfile and import it here.src/lib/tests/BrowserRow.test.js#L9-L15: Remove this duplicate and import the shared helper.src/lib/tests/Button.test.js#L16-L22: Remove this duplicate, import the shared helper, and adjust call sites to append.toJSON().src/lib/tests/ConfigConflictDiff.test.js#L8-L14: Remove this duplicate and import the shared helper.src/lib/tests/ScriptResponseModal.test.js#L33-L39: Remove this duplicate and import the shared helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/tests/BrowserCell.test.js` around lines 16 - 22, Centralize the duplicated renderComponent helper in a shared test-utils.js file, preserving its act-wrapped renderer.create behavior, then remove the local copies and import the shared helper in src/lib/tests/BrowserCell.test.js#L16-L22, src/lib/tests/BrowserRow.test.js#L9-L15, src/lib/tests/Button.test.js#L16-L22, src/lib/tests/ConfigConflictDiff.test.js#L8-L14, and src/lib/tests/ScriptResponseModal.test.js#L33-L39; update Button.test.js call sites to append .toJSON(), and leave any renderToJSON usage consistent with the shared utility as applicable.src/dashboard/index.js (1)
17-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReorder
importstatements aboverequire().Your reliance on hoisting for module imports is sloppy. ES6
importstatements are hoisted and executed before other code, so placing them afterrequire()calls or other statements creates a misleading execution order and violates standard linting rules.Move the
importstatements to the top of the file, above therequire()call, to ensure execution order is transparent.♻️ Proposed refactor
-require('stylesheets/fonts.scss'); -// GraphiQL 5 uses Monaco; register its web workers once before the app mounts. -import 'graphiql/setup-workers/webpack'; -import 'graphiql/style.css'; +// GraphiQL 5 uses Monaco; register its web workers once before the app mounts. +import 'graphiql/setup-workers/webpack'; +import 'graphiql/style.css'; + +require('stylesheets/fonts.scss');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/dashboard/index.js` around lines 17 - 20, Move the `graphiql/setup-workers/webpack` and `graphiql/style.css` import statements above the `require('stylesheets/fonts.scss')` call in `src/dashboard/index.js`, keeping the existing imports unchanged and placing all ES6 imports at the top of the file.src/dashboard/Data/ApiConsole/GraphQLConsole.react.js (1)
22-29: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueAvoid mutating instance properties during render.
You are mutating
this._fetcherKeyandthis._fetcherdirectly in therenderpath (viathis.getFetcher()). This violates React's pure render rules. Under React 19's concurrent rendering,rendermay be interrupted or invoked multiple times before committing, leading to race conditions or inconsistent cache states.Refactor this to derive the fetcher safely, either by migrating to a function component and using
useMemo, or by updating state usingcomponentDidUpdate/getDerivedStateFromProps.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/dashboard/Data/ApiConsole/GraphQLConsole.react.js` around lines 22 - 29, Refactor GraphQLConsole’s getFetcher flow so render no longer mutates _fetcherKey or _fetcher. Derive the fetcher with useMemo if converting to a function component, or maintain it through state synchronized in componentDidUpdate/getDerivedStateFromProps, while preserving URL and parsed-header cache invalidation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/dashboard/Data/Browser/Browser.react.js`:
- Line 335: Move the schema fetch at
src/dashboard/Data/Browser/Browser.react.js:335 and the refresh logic at
src/dashboard/Data/Browser/Browser.react.js:433 out of the removed legacy
lifecycle methods into the appropriate componentDidMount/componentDidUpdate
methods, preserving their existing conditions and behavior; alternatively,
rename both lifecycle methods to their UNSAFE_ variants as a stopgap.
In `@src/lib/tests/e2e/dashboard.e2e.test.js`:
- Around line 91-96: Update the teardown in the test’s finally block to isolate
browser and server cleanup with nested finally blocks, ensuring server.close()
is attempted even when browser.close() fails. Await the server close callback
before completing teardown so the Express server is deterministically stopped.
---
Outside diff comments:
In `@src/components/Popover/Popover.react.js`:
- Around line 87-111: Replace the disconnected-target early return in
_checkExternalClick with event-path inspection using e.composedPath(),
preserving outside-click detection even when targets detach after dispatch. Use
the composed path to determine whether the popover wrapper or inner popover was
traversed, and replace the direct e.target.parentNode.classList access with a
null-safe path-based chromeDropdown check so html or disconnected targets cannot
cause a TypeError.
---
Nitpick comments:
In `@src/dashboard/Data/ApiConsole/GraphQLConsole.react.js`:
- Around line 22-29: Refactor GraphQLConsole’s getFetcher flow so render no
longer mutates _fetcherKey or _fetcher. Derive the fetcher with useMemo if
converting to a function component, or maintain it through state synchronized in
componentDidUpdate/getDerivedStateFromProps, while preserving URL and
parsed-header cache invalidation.
In `@src/dashboard/index.js`:
- Around line 17-20: Move the `graphiql/setup-workers/webpack` and
`graphiql/style.css` import statements above the
`require('stylesheets/fonts.scss')` call in `src/dashboard/index.js`, keeping
the existing imports unchanged and placing all ES6 imports at the top of the
file.
In `@src/lib/tests/BrowserCell.test.js`:
- Around line 16-22: Centralize the duplicated renderComponent helper in a
shared test-utils.js file, preserving its act-wrapped renderer.create behavior,
then remove the local copies and import the shared helper in
src/lib/tests/BrowserCell.test.js#L16-L22,
src/lib/tests/BrowserRow.test.js#L9-L15, src/lib/tests/Button.test.js#L16-L22,
src/lib/tests/ConfigConflictDiff.test.js#L8-L14, and
src/lib/tests/ScriptResponseModal.test.js#L33-L39; update Button.test.js call
sites to append .toJSON(), and leave any renderToJSON usage consistent with the
shared utility as applicable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d1d5a41d-19fa-4038-b1aa-a9c7e7ad1eff
📒 Files selected for processing (38)
.gitignorepackage-lock.jsonpackage.jsonsrc/components/CategoryList/CategoryList.react.jssrc/components/ColumnsConfiguration/ColumnConfigurationItem.react.jssrc/components/DataBrowserHeader/DataBrowserHeader.react.jssrc/components/FileTree/FileTree.react.jssrc/components/Popover/Popover.react.jssrc/components/Sidebar/AppsMenu.react.jssrc/components/Sidebar/SidebarHeader.react.jssrc/components/Sidebar/SidebarSection.react.jssrc/components/Sidebar/SidebarSubItem.react.jssrc/components/Toolbar/Toolbar.react.jssrc/dashboard/AppData.react.jssrc/dashboard/Apps/AppsIndex.react.jssrc/dashboard/Dashboard.jssrc/dashboard/Data/ApiConsole/ApiConsole.react.jssrc/dashboard/Data/ApiConsole/GraphQLConsole.react.jssrc/dashboard/Data/Browser/Browser.react.jssrc/dashboard/Data/Jobs/JobsData.react.jssrc/dashboard/Data/Playground/Playground.react.jssrc/dashboard/Push/PushDetails.react.jssrc/dashboard/Settings/GeneralSettings.react.jssrc/dashboard/index.jssrc/lib/stores/StoreManager.jssrc/lib/tests/BrowserCell.test.jssrc/lib/tests/BrowserRow.test.jssrc/lib/tests/Button.test.jssrc/lib/tests/ConfigConflictDiff.test.jssrc/lib/tests/ScriptResponseModal.test.jssrc/lib/tests/e2e/dashboard.e2e.test.jssrc/lib/withRouter.jssrc/login/index.jssrc/parse-interface-guide/PIG.react.jssrc/parse-interface-guide/index.jssrc/parse-interface-guide/routes.jssrc/quickstart/index.jswebpack/base.config.js
Member
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- relevant file outlines ---'
ast-grep outline src/dashboard/Push/PushDetails.react.js --items all --type function,method,class --view expanded || true
ast-grep outline src/components/Sidebar/AppsMenu.react.js --items all --type function,class --view expanded || true
ast-grep outline src/lib/htmlString.js --items all --type function --view expanded || true
echo '--- PushDetails cited ranges ---'
sed -n '450,505p' src/dashboard/Push/PushDetails.react.js
echo '--- AppsMenu imports and Link usage ---'
sed -n '1,55p' src/components/Sidebar/AppsMenu.react.js
echo '--- htmlString implementation ---'
fd -a '^htmlString\.(js|jsx)$' src | while IFS= read -r f; do
echo "### $f"
cat -n "$f"
done
echo '--- isolated fix commit ---'
git show --format=fuller --stat --oneline be8f5387
git show --format= --find-renames -- src/dashboard/Push/PushDetails.react.js be8f5387
echo '--- React test renderer declarations and active test imports ---'
rg -n -C2 '"react-test-renderer"|react-test-renderer' package.json src/lib/testsLength of output: 10272
Would you like me to open a follow-up GitHub issue for migrating the remaining ✏️ Learnings added
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/stores/StoreManager.js (1)
50-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReturn
newStatefrom the synchronous dispatch branch.You must return
newStatefrom the synchronous branch ofdispatchbelow. It is inconsistent to return the updated state only when the action is asynchronous. ReturningnewStateguarantees that callers can always access the freshest state regardless of the action type.♻️ Proposed refactor
for (const id in storeData.subscribers) { storeData.subscribers[id](newState); } + return newState; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/stores/StoreManager.js` around lines 50 - 53, Update the synchronous branch of dispatch in StoreManager so it returns newState after applying the action, matching the asynchronous branch’s return value and ensuring callers always receive the freshest state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/dashboard/Data/Browser/Browser.react.js`:
- Line 1864: Handle all three Promise chains in Browser.react.js: update the
chain at lines 1864-1864 to return the chained promise while preserving the
fetched data for callers, and append rejection handlers to the chains at lines
335-335 and 433-433 that log errors with console.error to prevent unhandled
rejections.
In `@src/lib/tests/e2e/dashboard.e2e.test.js`:
- Around line 101-105: Update the server teardown in the finally block to call
server.closeAllConnections() before awaiting server.close(). Preserve the
existing close callback’s rejection and resolution behavior while ensuring
lingering connections are forcibly terminated.
---
Nitpick comments:
In `@src/lib/stores/StoreManager.js`:
- Around line 50-53: Update the synchronous branch of dispatch in StoreManager
so it returns newState after applying the action, matching the asynchronous
branch’s return value and ensuring callers always receive the freshest state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 57f1351c-b070-462e-ad0b-ee15308a1ada
📒 Files selected for processing (39)
.gitignorepackage-lock.jsonpackage.jsonsrc/components/CategoryList/CategoryList.react.jssrc/components/ColumnsConfiguration/ColumnConfigurationItem.react.jssrc/components/DataBrowserHeader/DataBrowserHeader.react.jssrc/components/FileTree/FileTree.react.jssrc/components/Popover/Popover.react.jssrc/components/Sidebar/AppsMenu.react.jssrc/components/Sidebar/SidebarHeader.react.jssrc/components/Sidebar/SidebarSection.react.jssrc/components/Sidebar/SidebarSubItem.react.jssrc/components/Toolbar/Toolbar.react.jssrc/dashboard/AppData.react.jssrc/dashboard/Apps/AppsIndex.react.jssrc/dashboard/Dashboard.jssrc/dashboard/Data/ApiConsole/ApiConsole.react.jssrc/dashboard/Data/ApiConsole/GraphQLConsole.react.jssrc/dashboard/Data/Browser/Browser.react.jssrc/dashboard/Data/Jobs/JobsData.react.jssrc/dashboard/Data/Playground/Playground.react.jssrc/dashboard/Push/PushDetails.react.jssrc/dashboard/Settings/GeneralSettings.react.jssrc/dashboard/index.jssrc/lib/stores/StoreManager.jssrc/lib/tests/BrowserCell.test.jssrc/lib/tests/BrowserRow.test.jssrc/lib/tests/Button.test.jssrc/lib/tests/ConfigConflictDiff.test.jssrc/lib/tests/ScriptResponseModal.test.jssrc/lib/tests/e2e/dashboard.e2e.test.jssrc/lib/tests/renderWithAct.jssrc/lib/withRouter.jssrc/login/index.jssrc/parse-interface-guide/PIG.react.jssrc/parse-interface-guide/index.jssrc/parse-interface-guide/routes.jssrc/quickstart/index.jswebpack/base.config.js
- Browser: attach a .catch to the fire-and-forget FETCH dispatch chains and return the chained promise from handleCLPChange (instead of a discarded side-effect chain), so a dispatch rejection surfaces via the caller instead of leaking as an unhandled rejection. - e2e: call server.closeAllConnections() (when available) before server.close() so teardown can't hang on a socket left open by a failed browser close.
Member
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/parse-interface-guide/index.js (1)
10-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStop conflating React components with instantiated React elements.
You are clearly relying on a flawed heuristic that all React imports must be capitalized. The
./routesmodule exports an instantiated JSX element (<BrowserRouter>...</BrowserRouter>), not a component function. Naming itRouteswith a capital "R" falsely signals to human developers that it is a component. Lowercase this import toroutesto reflect reality and prevent cognitive dissonance when readingroot.render(routes).♻️ Proposed refactor
-import Routes from './routes'; +import routes from './routes'; // App entry point const root = createRoot(document.getElementById('browser_mount')); -root.render(Routes); +root.render(routes);Also applies to: 14-15
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/parse-interface-guide/index.js` at line 10, Rename the imported JSX element from Routes to routes in the import declaration and update all references in the affected render flow, including root.render, so the instantiated element is consistently lowercase while preserving behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/parse-interface-guide/index.js`:
- Line 10: Rename the imported JSX element from Routes to routes in the import
declaration and update all references in the affected render flow, including
root.render, so the instantiated element is consistently lowercase while
preserving behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1606ef01-2e68-4ee3-9e8c-529ac1900abf
📒 Files selected for processing (39)
.gitignorepackage-lock.jsonpackage.jsonsrc/components/CategoryList/CategoryList.react.jssrc/components/ColumnsConfiguration/ColumnConfigurationItem.react.jssrc/components/DataBrowserHeader/DataBrowserHeader.react.jssrc/components/FileTree/FileTree.react.jssrc/components/Popover/Popover.react.jssrc/components/Sidebar/AppsMenu.react.jssrc/components/Sidebar/SidebarHeader.react.jssrc/components/Sidebar/SidebarSection.react.jssrc/components/Sidebar/SidebarSubItem.react.jssrc/components/Toolbar/Toolbar.react.jssrc/dashboard/AppData.react.jssrc/dashboard/Apps/AppsIndex.react.jssrc/dashboard/Dashboard.jssrc/dashboard/Data/ApiConsole/ApiConsole.react.jssrc/dashboard/Data/ApiConsole/GraphQLConsole.react.jssrc/dashboard/Data/Browser/Browser.react.jssrc/dashboard/Data/Jobs/JobsData.react.jssrc/dashboard/Data/Playground/Playground.react.jssrc/dashboard/Push/PushDetails.react.jssrc/dashboard/Settings/GeneralSettings.react.jssrc/dashboard/index.jssrc/lib/stores/StoreManager.jssrc/lib/tests/BrowserCell.test.jssrc/lib/tests/BrowserRow.test.jssrc/lib/tests/Button.test.jssrc/lib/tests/ConfigConflictDiff.test.jssrc/lib/tests/ScriptResponseModal.test.jssrc/lib/tests/e2e/dashboard.e2e.test.jssrc/lib/tests/renderWithAct.jssrc/lib/withRouter.jssrc/login/index.jssrc/parse-interface-guide/PIG.react.jssrc/parse-interface-guide/index.jssrc/parse-interface-guide/routes.jssrc/quickstart/index.jswebpack/base.config.js
./routes exports an instantiated <BrowserRouter> element, not a component, so importing it as Routes (and colliding with react-router's Routes) misleads; name it routes to match what it is.
Member
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Member
Author
|
The remaining nitpick ( |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/dashboard/index.js`:
- Around line 17-21: Move the GraphiQL worker and stylesheet imports above the
stylesheets/fonts.scss require in the module so all import declarations appear
before require-based initialization. Preserve the existing import contents and
installDevTools(Immutable) behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72696b35-724c-4cf4-a5b1-7227d27032c2
📒 Files selected for processing (39)
.gitignorepackage-lock.jsonpackage.jsonsrc/components/CategoryList/CategoryList.react.jssrc/components/ColumnsConfiguration/ColumnConfigurationItem.react.jssrc/components/DataBrowserHeader/DataBrowserHeader.react.jssrc/components/FileTree/FileTree.react.jssrc/components/Popover/Popover.react.jssrc/components/Sidebar/AppsMenu.react.jssrc/components/Sidebar/SidebarHeader.react.jssrc/components/Sidebar/SidebarSection.react.jssrc/components/Sidebar/SidebarSubItem.react.jssrc/components/Toolbar/Toolbar.react.jssrc/dashboard/AppData.react.jssrc/dashboard/Apps/AppsIndex.react.jssrc/dashboard/Dashboard.jssrc/dashboard/Data/ApiConsole/ApiConsole.react.jssrc/dashboard/Data/ApiConsole/GraphQLConsole.react.jssrc/dashboard/Data/Browser/Browser.react.jssrc/dashboard/Data/Jobs/JobsData.react.jssrc/dashboard/Data/Playground/Playground.react.jssrc/dashboard/Push/PushDetails.react.jssrc/dashboard/Settings/GeneralSettings.react.jssrc/dashboard/index.jssrc/lib/stores/StoreManager.jssrc/lib/tests/BrowserCell.test.jssrc/lib/tests/BrowserRow.test.jssrc/lib/tests/Button.test.jssrc/lib/tests/ConfigConflictDiff.test.jssrc/lib/tests/ScriptResponseModal.test.jssrc/lib/tests/e2e/dashboard.e2e.test.jssrc/lib/tests/renderWithAct.jssrc/lib/withRouter.jssrc/login/index.jssrc/parse-interface-guide/PIG.react.jssrc/parse-interface-guide/index.jssrc/parse-interface-guide/routes.jssrc/quickstart/index.jswebpack/base.config.js
…ard entry
The graphiql side-effect imports hoist above require('stylesheets/fonts.scss')
regardless of placement; move them up with the other imports so the written
order matches evaluation order. fonts.scss is only Google-Font @imports, so the
cascade is unaffected.
Member
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/dashboard/Push/PushDetails.react.js (1)
488-502: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix the glaring typo in the React key.
You have explicitly hardcoded
key="sublime_2"in two separate branches here, while your other siblings usesubline_0,subline_1, andsubline_3. Relying on a heuristic like "it's unique so it works" is a poor excuse for shipping glaring typos in your React keys. Fix it so it properly readssubline_2and aligns with the rest of the array's sequence.🛠️ Proposed typo fix
if (pushDetails.launch_info && pushDetails.launch_info.recipient_count) { subline.push( - <div key="sublime_2"> + <div key="subline_2"> Your Launch Group is <strong>{pushDetails.launch_info.percent}%</strong> (approximately{' '} <strong>{prettyNumber(pushDetails.launch_info.recipient_count)}</strong>) of the devices in this campaign. {launchGroupFragment} </div> ); } else { subline.push( - <div key="sublime_2"> + <div key="subline_2"> Your Launch Group is <strong>{pushDetails.launch_info.percent}%</strong> of the devices in this campaign. {launchGroupFragment} </div> ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/dashboard/Push/PushDetails.react.js` around lines 488 - 502, Correct the hardcoded React key in both branches of the launch-group rendering within the subline construction so it uses “subline_2” instead of the misspelled “sublime_2”, preserving the existing key sequence.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/tests/e2e/dashboard.e2e.test.js`:
- Around line 75-78: Configure Puppeteer operations in the dashboard test’s
setup around browser.launch and page.goto with explicit timeouts below the
20-second Jest timeout, such as 15 seconds, so failures reject before Jest
aborts and the existing finally cleanup executes.
---
Nitpick comments:
In `@src/dashboard/Push/PushDetails.react.js`:
- Around line 488-502: Correct the hardcoded React key in both branches of the
launch-group rendering within the subline construction so it uses “subline_2”
instead of the misspelled “sublime_2”, preserving the existing key sequence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c7cfd6f4-e0b3-414b-a7d5-a01b38a08644
📒 Files selected for processing (39)
.gitignorepackage-lock.jsonpackage.jsonsrc/components/CategoryList/CategoryList.react.jssrc/components/ColumnsConfiguration/ColumnConfigurationItem.react.jssrc/components/DataBrowserHeader/DataBrowserHeader.react.jssrc/components/FileTree/FileTree.react.jssrc/components/Popover/Popover.react.jssrc/components/Sidebar/AppsMenu.react.jssrc/components/Sidebar/SidebarHeader.react.jssrc/components/Sidebar/SidebarSection.react.jssrc/components/Sidebar/SidebarSubItem.react.jssrc/components/Toolbar/Toolbar.react.jssrc/dashboard/AppData.react.jssrc/dashboard/Apps/AppsIndex.react.jssrc/dashboard/Dashboard.jssrc/dashboard/Data/ApiConsole/ApiConsole.react.jssrc/dashboard/Data/ApiConsole/GraphQLConsole.react.jssrc/dashboard/Data/Browser/Browser.react.jssrc/dashboard/Data/Jobs/JobsData.react.jssrc/dashboard/Data/Playground/Playground.react.jssrc/dashboard/Push/PushDetails.react.jssrc/dashboard/Settings/GeneralSettings.react.jssrc/dashboard/index.jssrc/lib/stores/StoreManager.jssrc/lib/tests/BrowserCell.test.jssrc/lib/tests/BrowserRow.test.jssrc/lib/tests/Button.test.jssrc/lib/tests/ConfigConflictDiff.test.jssrc/lib/tests/ScriptResponseModal.test.jssrc/lib/tests/e2e/dashboard.e2e.test.jssrc/lib/tests/renderWithAct.jssrc/lib/withRouter.jssrc/login/index.jssrc/parse-interface-guide/PIG.react.jssrc/parse-interface-guide/index.jssrc/parse-interface-guide/routes.jssrc/quickstart/index.jswebpack/base.config.js
…n test launch/goto/waitForSelector defaulted to 30s while the Jest test timeout is 20s, so a hang would let Jest abort before the finally block runs, leaking the Express server. Cap launch/goto at 15s and waitForSelector at 10s so any hang rejects in time for cleanup.
Member
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
parseplatformorg
pushed a commit
that referenced
this pull request
Jul 18, 2026
# [9.3.0-alpha.1](9.2.1-alpha.1...9.3.0-alpha.1) (2026-07-18) ### Features * React 19 + react-router 7 + GraphiQL 5 coordinated upgrade ([#3409](#3409)) ([c3e16c2](c3e16c2))
This was referenced Jul 18, 2026
Contributor
|
🎉 This change has been released in version 9.3.0-alpha.1 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Coordinated upgrade of the four tightly-coupled majors from #3408, landed on one branch as reviewable, bisectable commits:
createRoot, and remediation of third-party deps that block React 19.react-routerpackage;react-router-domremoved.createGraphiQLFetcher, code-split behindReact.lazy.Closes #3408.
Supersedes #3336, #3341, #3369, #3372, #3377 (the deferred Dependabot bumps — this PR replaces them; leaving them open for the maintainer to close on merge).
Commits (one per step, each independently green)
feat: React 16→18—createRoot×3; replacereact-json-view(React ≤17 cap) with@microlink/react-json-view; migratereact-dnd10→16 (decorators→hooks inDataBrowserHeader, v16 hooks API inColumnConfigurationItem).feat: React 18→19— bump to 19.2.7;react-draggable/react-resizablebumps forfindDOMNoderemoval; lazy-loadGraphQLConsole; webpackresolve.moduleswalk-up;act()-wrap the 5react-test-renderersuites.chore: v7 future flags+feat: react-router 6→7— enable flags, then flip all 19 importers toreact-routerand dropreact-router-dom.feat: GraphiQL 2→5— graphiql 5.2.4 +@graphiql/toolkit+graphql-ws;setup-workers/webpack;.ttfasset rule;output.publicPath: 'auto'.Notable, non-obvious changes
@babel/plugin-proposal-decoratorsis kept —@withRouter/@subscribeToare class decorators used across ~25 files.createRootexposed two latent React-18 automatic-batching bugs in the Data Browser (state read right aftersetStatein a promise); fixed by using resolved values, incl. makingStoreManager.dispatchresolve with the new state.GraphQLConsoleis lazy-loaded — graphiql@2 read the React-19-removedReactCurrentOwnerat module load and crashed the whole app on boot; lazy-loading isolates it and permanently code-splits the Monaco chunk.output.publicPath: 'auto'— required so Monaco's emitted*.worker.jschunks resolve at runtime (the relativebundles/doubled tobundles/bundles/).monaco-editorstays transitive at 0.52.2 (pinned by@graphiql/react; not added directly).Design spec & implementation notes:
docs/superpowers/specs/2026-07-17-react19-router7-graphiql5-migration-design.md(§8).Verification
npm run lint,npm test(264 passing, incl. the puppeteer e2e),npm run build(dashboard + PIG), and thepreparepublish build — all green from a cleannpm ci./appsredirect, Data Browser (columns, drag, resize), Playground JSON output, Canvas, routing (deep links,cloud_code/*splat, 404, back/forward), and 14 dashboard screens — no crash-type console/page errors.X-Parse-*headers.🤖 Generated with Claude Code
Summary by CodeRabbit
act()helper.