-
Notifications
You must be signed in to change notification settings - Fork 373
5th: react-router upgrade to react-router-dom
#9134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
camd
wants to merge
32
commits into
master
Choose a base branch
from
camd/react-router-upgrade
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
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
59583ba to
c64c6a8
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #9134 +/- ##
==========================================
+ Coverage 81.43% 82.21% +0.78%
==========================================
Files 601 605 +4
Lines 33793 34030 +237
Branches 3067 3280 +213
==========================================
+ Hits 27520 27979 +459
+ Misses 6118 5720 -398
- Partials 155 331 +176 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cd39d89 to
9c5f93f
Compare
- Add tests/ui/App.test.jsx: Test URL transformations and routing - URL backwards compatibility (perf.html, pushhealth.html, logviewer.html) - Permalink hash preservation - Document title updates for perfherder alerts - Export verification (no react-hot-loader wrapper) - Expand tests/ui/helpers/filter.test.js: Comprehensive filter helper tests - thMatchType, thFieldChoices, thFilterDefaults constants - arraysEqual and matchesDefaults functions - hasUrlFilterChanges URL comparison - reloadOnChangeParameters and allFilterParams arrays - Add tests/ui/perfherder/App.test.jsx: Export and structure verification - Verify no hot loader wrapper (RSPack migration regression test) - Validate class component structure and lifecycle methods - Add tests/ui/push-health/App.test.jsx: Component and routing tests - Component loading and API call verification - Route handling (push, my-pushes, usage) - Export verification for RSPack migration 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Change server command from 'yarn start' to 'pnpm start' to match the package manager migration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add tests for dayjs helper module (UTC, customParseFormat, isSameOrAfter, relativeTime plugins) - Add tests for display.js dayjs migration (toMercurialDateStr, toMercurialShortDateStr) - Add tests for taskcluster.js credential expiration with dayjs - Add tests for DateRangePicker MUI component migration - Add tests for intermittent-failures helpers (prettyDate, calculateMetrics) - Add tests for perfherder helpers (getFilledBugSummary date formatting) - Add tests for GraphsContainer and TableView date formatting - Add tests for job-view App react-resizable-panels migration 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add format:md, format:md:check, and lint:md scripts using prettier - Fix lint errors in test files: - taskcluster.test.js: Use toThrow() instead of try/catch - DateRangePicker.test.jsx: Remove jest.mock with require(), use toHaveLength() - helpers.test.jsx: Use toHaveLength() - GraphsContainer.test.jsx: Remove unused variable 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add tests/ui/App.test.jsx: Test URL transformations and routing - URL backwards compatibility (perf.html, pushhealth.html, logviewer.html) - Permalink hash preservation - Document title updates for perfherder alerts - Export verification (no react-hot-loader wrapper) - Expand tests/ui/helpers/filter.test.js: Comprehensive filter helper tests - thMatchType, thFieldChoices, thFilterDefaults constants - arraysEqual and matchesDefaults functions - hasUrlFilterChanges URL comparison - reloadOnChangeParameters and allFilterParams arrays - Add tests/ui/perfherder/App.test.jsx: Export and structure verification - Verify no hot loader wrapper (RSPack migration regression test) - Validate class component structure and lifecycle methods - Add tests/ui/push-health/App.test.jsx: Component and routing tests - Component loading and API call verification - Route handling (push, my-pushes, usage) - Export verification for RSPack migration 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Change server command from 'yarn start' to 'pnpm start' to match the package manager migration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…use safer approach.
Add comprehensive test coverage for changes introduced in the defaultProps deprecation fixes: - tests/ui/hooks/useDebounce.test.js: Tests for useDebouncedValue and useDebouncedCallback hooks (17 tests) - tests/ui/hooks/useJobButtonRegistry.test.js: Tests for job button registry functions and hook lifecycle (25 tests) - tests/ui/helpers/job.test.js: Tests for getBtnClass, isReftest, isPerfTest, canConfirmFailure, isClassified, isUnclassifiedFailure, findInstance, getResultState, addAggregateFields, getTaskRunStr, getTaskRun (57 tests) - tests/ui/job-view/pushes/JobButton.test.jsx: Tests for the converted functional JobButton component (23 tests) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace require() with ES module import in job.test.js to fix global-require - Use shorthand boolean attributes (visible, intermittent) in JobButton.test.jsx 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
487d2e1 to
aa37f61
Compare
The withView HOC is a class component that expected location and history props to be passed automatically by React Router. In React Router v6, these are no longer passed - you must use hooks instead. This caused the error: "Cannot read properties of undefined (reading 'state')" at line 21: this.default = this.props.location.state || defaultState; Fix by creating a ViewWithRouter wrapper component that: - Uses useLocation() and useNavigate() hooks - Injects location and navigate as props to the class component - Creates a history-like object with push/replace for backwards compatibility Also adds unit tests to verify the intermittent-failures routes render correctly with React Router v6. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace the withView class-based Higher Order Component with a modern useIntermittentFailuresData custom hook. This eliminates the need for the ViewWithRouter wrapper and makes the code more readable and testable. Changes: - Create useIntermittentFailuresData.jsx custom hook that encapsulates all state management and data fetching logic - Update MainView.jsx to use the hook directly instead of withView HOC - Update BugDetailsView.jsx to use the hook directly instead of withView HOC - Remove View.jsx (the old class-based HOC) - Update tests to work with the new hook-based implementation The hook uses: - useState for state management (replacing this.state) - useEffect for lifecycle (replacing componentDidMount) - useCallback for memoized functions - useRef for pending update tracking (replacing setState callbacks) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
aa37f61 to
a770a26
Compare
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.
Build on top of #9124
React Router v6 Migration Summary
Overview
This PR migrates from
react-router-domv5.1.2 to v6.x and removes the deprecatedconnected-react-routerlibrary.Key Changes
Dependencies
API Changes
<Switch><Routes><Route render={() => <Comp />} /><Route element={<Comp />} /><Redirect to="/path" /><Route element={<Navigate to="/path" />} />useHistory()/history.push()useNavigate()/navigate()props.match.pathConnectedRouterBrowserRouterFiles Modified
Redux Store Changes:
ui/job-view/redux/configureStore.js- Removed router middleware and reducerRouter Updates:
ui/App.jsx- ReplacedConnectedRouter+SwitchwithBrowserRouter+Routesui/push-health/App.jsx- Updated to v6 routing patternsui/perfherder/App.jsx- Updated to v6 routing patternsui/intermittent-failures/App.jsx- Updated to v6 routing patternsNavigation Refactoring:
ui/job-view/App.jsx- ReplacedpushRoutewithuseNavigateui/job-view/headerbars/SecondaryNavBar.jsx- ReplacedpushRoutewithuseNavigateui/job-view/pushes/PushActionMenu.jsx- ReplacedpushRoutewithuseNavigateui/job-view/redux/stores/selectedJob.js- Useswindow.history.pushStatefor URL updatesui/job-view/redux/stores/pushes.js- Useswindow.history.pushStatefor URL updatesui/models/filter.js- Accepts navigate function instead of Redux pushRouteNew Utilities:
ui/helpers/router.js- Centralized navigation hooks and utilitiesWhy This Migration?
connected-react-routeris deprecated and unmaintainedTesting