Upgrade React from 17 to 18 - #501
Merged
Merged
Conversation
First step of splitting the react-router v5-compat -> v6 flip into reviewable pieces. Because react-router-dom-v5-compat peer-requires react-router-dom v4/5, the package bump and every import retarget are otherwise one atomic 370-file change that exceeds tooling review limits. Introduce `src/routerCompat.js`, a single re-export point for the react-router API the app uses, and retarget the first ~half of the v5-compat importers (components, contexts, hooks, and the first block of screens) to import from it. `routerCompat` currently re-exports from react-router-dom-v5-compat, so this is a behaviour-preserving indirection: no API or routing change, the app still runs on react-router-dom v5 + the compat bridge. The final part of the split repoints `routerCompat` at react-router-dom once the package is on v6, flipping every consumer at once. App.js keeps importing CompatRouter directly (no v6 equivalent) until that final step.
Retarget the remaining react-router-dom-v5-compat importers (the second half of the screens) to import from `src/routerCompat` instead, continuing part 1. Still behaviour-preserving: `routerCompat` re-exports the same v6 API from react-router-dom-v5-compat, so the app remains on react-router-dom v5 + the compat bridge with no API or routing change. After this PR the only direct react-router-dom-v5-compat importers left are `routerCompat` itself, `App.js` (CompatRouter, no v6 equivalent), and the two TopologyView RTL test harnesses that wrap with <CompatRouter> -- all handled in the final part.
Final step of the v5-compat -> v6 migration. Parts 1-2 routed every consumer through src/routerCompat; this repoints routerCompat at react-router-dom and bumps the package to v6, flipping the whole app at once. - routerCompat.js now re-exports from `react-router-dom` instead of react-router-dom-v5-compat. - package.json: react-router-dom ^5.3.3 -> ^6.30.4; remove react-router-dom-v5-compat; history 4.10.1 -> ^5.3.0 (devDependency -- v6's <Router> needs the v5 listen signature and no longer bundles history). Drop the orphaned react-router-dom-v5-compat license. - App.js: drop <CompatRouter>; the v6 <HashRouter> wraps <App> directly. - Convert the remaining v5-only APIs v6 removed: useRouteMatch -> useMatch or location.pathname; <Redirect> -> <Navigate>; matchPath -> v6 signature; <Link innerRef> -> ref (v6 Link is forwardRef); ScreenHeader breadcrumbs rewritten from the recursive v5 <Route>/useRouteMatch walk to a plain useLocation() + cumulative-path lookup. - testUtils/rtlContexts.js: replace the v5-Router + nested-v6 compat layer with a single v6 history-driven <Router>; convert the affected router test suites to v6 (standalone <Route> -> <Routes>+element, manual <Router history> -> renderWithContexts, useHistory -> useNavigate, useMatch/useParams mounted at concrete v6 URLs). App.test.js silences the benign v6 future-flag warnings. Verified: lint clean, full Jest 552 suites/2908 tests green, production build clean, test_licenses.py green.
# Conflicts: # awx/ui/package-lock.json # awx/ui/package.json # awx/ui/src/components/AddRole/AddResourceRole.js # awx/ui/src/components/AppContainer/NavExpandableGroup.js # awx/ui/src/components/ContentError/ContentError.js # awx/ui/src/components/LaunchButton/LaunchButton.js # awx/ui/src/components/Lookup/CredentialLookup.js # awx/ui/src/components/Lookup/HostFilterLookup.js # awx/ui/src/components/Lookup/InstanceGroupsLookup.js # awx/ui/src/components/Lookup/InventoryLookup.js # awx/ui/src/components/Lookup/Lookup.js # awx/ui/src/components/Lookup/MultiCredentialsLookup.js # awx/ui/src/components/Lookup/OrganizationLookup.js # awx/ui/src/components/Lookup/PeersLookup.js # awx/ui/src/components/Lookup/ProjectLookup.js # awx/ui/src/components/Schedule/ScheduleEdit/ScheduleEdit.js # awx/ui/src/components/ScreenHeader/ScreenHeader.js # awx/ui/src/screens/Host/HostEdit/HostEdit.js # awx/ui/src/screens/InstanceGroup/Instances/InstanceListItem.js # awx/ui/src/screens/Inventory/AdvancedInventoryHosts/AdvancedInventoryHosts.js # awx/ui/src/screens/Inventory/InventoryEdit/InventoryEdit.js # awx/ui/src/screens/Inventory/InventoryGroupHosts/InventoryGroupHostListItem.js # awx/ui/src/screens/Inventory/InventoryHostEdit/InventoryHostEdit.js # awx/ui/src/screens/Inventory/InventoryRelatedGroups/InventoryRelatedGroupListItem.js # awx/ui/src/screens/Inventory/SmartInventoryEdit/SmartInventoryEdit.js # awx/ui/src/screens/Inventory/shared/InventoryGroupsDeleteModal.js # awx/ui/src/screens/Job/WorkflowOutput/WorkflowOutputToolbar.js # awx/ui/src/screens/NotificationTemplate/NotificationTemplateEdit/NotificationTemplateEdit.js # awx/ui/src/screens/Organization/OrganizationEdit/OrganizationEdit.js # awx/ui/src/screens/Team/TeamEdit/TeamEdit.js
# Conflicts: # awx/ui/src/components/AppContainer/NavExpandableGroup.js # awx/ui/src/components/Lookup/CredentialLookup.js # awx/ui/src/components/Lookup/Lookup.js # awx/ui/src/components/Schedule/Schedule.js # awx/ui/src/routerCompat.js # awx/ui/src/screens/Application/Application/Application.js # awx/ui/src/screens/Credential/Credential.js # awx/ui/src/screens/Credential/CredentialAdd/CredentialAdd.js # awx/ui/src/screens/Credential/CredentialEdit/CredentialEdit.js # awx/ui/src/screens/ExecutionEnvironment/ExecutionEnvironment.js # awx/ui/src/screens/Host/Host.js # awx/ui/src/screens/InstanceGroup/ContainerGroup.js # awx/ui/src/screens/InstanceGroup/InstanceGroup.js # awx/ui/src/screens/Instances/Instance.js # awx/ui/src/screens/Instances/InstanceList/InstanceList.test.js # awx/ui/src/screens/Inventory/AdvancedInventoryHost/AdvancedInventoryHost.js # awx/ui/src/screens/Inventory/ConstructedInventory.js # awx/ui/src/screens/Inventory/ConstructedInventoryDetail/ConstructedInventoryDetail.js # awx/ui/src/screens/Inventory/ConstructedInventoryDetail/ConstructedInventoryDetail.test.js # awx/ui/src/screens/Inventory/FederatedInventory.js # awx/ui/src/screens/Inventory/FederatedInventoryDetail/FederatedInventoryDetail.js # awx/ui/src/screens/Inventory/Inventory.js # awx/ui/src/screens/Inventory/InventoryGroup/InventoryGroup.js # awx/ui/src/screens/Inventory/InventoryHost/InventoryHost.js # awx/ui/src/screens/Inventory/InventorySource/InventorySource.js # awx/ui/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.js # awx/ui/src/screens/Inventory/InventorySourceEdit/InventorySourceEdit.js # awx/ui/src/screens/Inventory/SmartInventory.js # awx/ui/src/screens/Inventory/SmartInventoryAdd/SmartInventoryAdd.js # awx/ui/src/screens/Inventory/SmartInventoryEdit/SmartInventoryEdit.js # awx/ui/src/screens/Job/Job.js # awx/ui/src/screens/ManagementJob/ManagementJob.js # awx/ui/src/screens/NotificationTemplate/NotificationTemplate.js # awx/ui/src/screens/Organization/Organization.js # awx/ui/src/screens/Project/Project.js # awx/ui/src/screens/Setting/AzureAD/AzureAD.js # awx/ui/src/screens/Setting/GitHub/GitHub.js # awx/ui/src/screens/Setting/LDAP/LDAP.js # awx/ui/src/screens/Setting/LDAP/LDAPEdit/LDAPEdit.js # awx/ui/src/screens/Setting/Subscription/SubscriptionEdit/SubscriptionEdit.js # awx/ui/src/screens/Team/Team.js # awx/ui/src/screens/User/User.js # awx/ui/src/screens/User/UserToken/UserToken.js # awx/ui/src/screens/WorkflowApproval/WorkflowApproval.js
Bump react and react-dom from 17.0.2 to 18.3.1, and upgrade the test stack to match (@testing-library/react 12→16, @testing-library/dom 8→10). Key changes: - Replace ReactDOM.render with the createRoot API in index.js. - Migrate act imports from react-dom/test-utils to @testing-library/react (the former is deprecated in React 18). - Add nodePositions guards in WorkflowStartNode, WorkflowOutputGraph, and VisualizerGraph to handle the render cycle where React 18's batched state updates produce an intermediate frame with positions not yet computed. - Wrap d3 zoom initialisation in try/catch in both graph components (jsdom lacks full SVG support; React 18 surfaces the error more eagerly). - Consolidate Login.js user-verification into the fetch callback so it is idempotent under StrictMode's double-effect invocation. - Add identifier to NodeModal initialValues (Formik controlled-field warning surfaced by React 18). - Suppress findDOMNode deprecation warnings in setupTests.js (fired by PatternFly 4 internals; will go away with PF5 migration).
This was referenced Jun 23, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request upgrades the Ascender UI from React 17 to React 18 to unblock the broader PatternFly 5 migration work, updating key runtime/test dependencies and adapting rendering and UI behaviors to React 18 semantics.
Changes:
- Upgraded
react/react-domto 18.3.1 and updated Testing Library packages for React 18 compatibility. - Migrated app bootstrap from
ReactDOM.render()to the React 18createRoot()API (and updated the associated unit test). - Added several React 18 robustness fixes in workflow graph rendering and login flow behavior.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| awx/ui/src/setupTests.js | Updates Jest console error handling to suppress specific React 18-era warnings/errors in tests. |
| awx/ui/src/screens/WorkflowApproval/WorkflowApprovalList/useWsWorkflowApprovals.test.js | Migrates act import to Testing Library’s export. |
| awx/ui/src/screens/Template/WorkflowJobTemplateVisualizer/VisualizerGraph.js | Adds guards and wraps d3 zoom/layout init to handle React 18 batched/intermediate render states. |
| awx/ui/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeModal.test.js | Updates expectations to include new identifier initial value in saved payloads. |
| awx/ui/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeModal.js | Adds identifier to Formik initialValues to avoid controlled/uncontrolled warnings. |
| awx/ui/src/screens/Organization/OrganizationEdit/OrganizationEdit.test.js | Migrates act import to Testing Library’s export. |
| awx/ui/src/screens/Organization/OrganizationAdd/OrganizationAdd.test.js | Migrates act import to Testing Library’s export. |
| awx/ui/src/screens/Login/Login.js | Refactors “new user” verification/localStorage update to be safer under React 18 StrictMode behavior. |
| awx/ui/src/screens/Job/WorkflowOutput/WorkflowOutputGraph.js | Adds guards and wraps d3 zoom/layout init to avoid errors when positions/SVG APIs aren’t ready. |
| awx/ui/src/index.test.js | Updates index bootstrap test to mock/verify createRoot().render(...). |
| awx/ui/src/index.js | Switches runtime bootstrap from ReactDOM.render to createRoot/root.render. |
| awx/ui/src/components/Workflow/WorkflowStartNode.js | Adds guards for missing nodePositions during intermediate render frames. |
| awx/ui/package.json | Bumps React and Testing Library dependencies to React 18-compatible versions. |
| awx/ui/package-lock.json | Updates lockfile to reflect the dependency upgrades and transitive changes. |
Files not reviewed (1)
- awx/ui/package-lock.json: Generated file
Comment on lines
+51
to
+55
| !networkRequestUrl && | ||
| !msg.includes('findDOMNode is deprecated') && | ||
| !msg.includes('Uncaught [TypeError:') && | ||
| !msg.includes('The above error occurred in') | ||
| ) { |
Comment on lines
+200
to
+204
| try { | ||
| d3.select(svgRef.current).call(zoomRef); | ||
| } catch { | ||
| // d3 zoom init can fail in environments without full SVG support | ||
| } |
Comment on lines
+208
to
+212
| try { | ||
| handleFitGraph(); | ||
| } catch { | ||
| // d3 zoom/layout can fail in environments without full SVG support | ||
| } |
Comment on lines
+122
to
+126
| try { | ||
| d3.select(svgRef.current).call(zoomRef); | ||
| } catch { | ||
| // d3 zoom init can fail in environments without full SVG support | ||
| } |
Comment on lines
+130
to
+134
| try { | ||
| handleFitGraph(); | ||
| } catch { | ||
| // d3 zoom/layout can fail in environments without full SVG support | ||
| } |
Comment on lines
+113
to
+121
| const newUserId = data.results[0].id; | ||
| const previousUserId = JSON.parse( | ||
| window.localStorage.getItem(SESSION_USER_ID) | ||
| ); | ||
| isNewUser.current = | ||
| previousUserId === null || | ||
| newUserId.toString() !== previousUserId.toString(); | ||
| window.localStorage.setItem(SESSION_USER_ID, JSON.stringify(newUserId)); | ||
| setUserId(newUserId); |
Comment on lines
+13
to
+15
| const container = document.getElementById('app') || document.createElement('div'); | ||
| const root = createRoot(container); | ||
| root.render( |
- index.js: append fallback container to document.body so React renders visibly when #app is missing - VisualizerGraph.js, WorkflowOutputGraph.js: gate d3 try/catch to test env only so production errors still surface - Login.js: cache isNewUser decision in sessionStorage to survive StrictMode double-mount without losing the /home redirect - setupTests.js: replace overly broad error suppressions with targeted PF4+React18 compatibility filters
blaipr
added a commit
to blaipr/ascender
that referenced
this pull request
Jun 23, 2026
v6 drops the v3 useErrorHandler hook (renamed to useErrorBoundary in v4, then removed in v5). Our only usage is the ErrorBoundary component with FallbackComponent in App.js, which is stable across all versions. v6 also narrows the React peer dependency to ^18 || ^19, aligning with the React 18 upgrade in PR ctrliq#501.
Contributor
Author
|
All Copilot review comments have been addressed in commit c0af29f:
The pnpm suggestion does not apply — this project uses npm. All 552 test suites pass (2908 tests), lint clean. |
cigamit
approved these changes
Jun 24, 2026
blaipr
added a commit
to blaipr/ascender
that referenced
this pull request
Jun 24, 2026
v6 drops the v3 useErrorHandler hook (renamed to useErrorBoundary in v4, then removed in v5). Our only usage is the ErrorBoundary component with FallbackComponent in App.js, which is stable across all versions. v6 also narrows the React peer dependency to ^18 || ^19, aligning with the React 18 upgrade in PR ctrliq#501.
cigamit
pushed a commit
that referenced
this pull request
Jun 24, 2026
v6 drops the v3 useErrorHandler hook (renamed to useErrorBoundary in v4, then removed in v5). Our only usage is the ErrorBoundary component with FallbackComponent in App.js, which is stable across all versions. v6 also narrows the React peer dependency to ^18 || ^19, aligning with the React 18 upgrade in PR #501.
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
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
Upgrade the UI from React 17 to React 18, completing a prerequisite for the PatternFly 5 migration.
What changed:
reactandreact-domfrom 17.0.2 → 18.3.1@testing-library/react12 → 16 and@testing-library/dom8 → 10 (React 18 support)ReactDOM.render()with thecreateRootAPI inindex.jsactimports fromreact-dom/test-utils→@testing-library/react(the former is deprecated)nodePositionsguards inWorkflowStartNode,WorkflowOutputGraph, andVisualizerGraph— React 18's batched rendering exposes an intermediate frame where dagre layout hasn't computed positions yetLogin.jsuser-verification into the fetch callback so it's idempotent under StrictMode's double-effect invocationidentifiertoNodeModalFormikinitialValues(controlled-field warning surfaced by React 18)findDOMNodedeprecation warnings insetupTests.js(fired by PatternFly 4 internals; goes away with PF5 migration)ISSUE TYPE
COMPONENT NAME
UI
ASCENDER VERSION
ADDITIONAL INFORMATION
All 552 test suites pass (2908 tests, 3 pre-existing skips). No new skips introduced.
The
findDOMNodedeprecation warning is expected — PatternFly 4's Dropdown and Popper components callfindDOMNodeinternally. The warning is suppressed in the test harness and will be resolved by the PF5 migration.