Skip to content
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.

Sign up for GitHub

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

Jump to bottom

chore: update to use react-router@6+ #19184

Merged
merged 1 commit into from Jan 15, 2024
Merged

chore: update to use react-router@6+ #19184

merged 1 commit into from Jan 15, 2024

Conversation

joshuaellis
Copy link
Member

@joshuaellis joshuaellis commented Jan 9, 2024 •
edited

What does it do?

  • Updates react-router-dom to version 6
  • Migrates our code so the app still works e.g. using a data-router, changing hooks to non-removed version
  • adds deprecations for adding settings or menu to properties with absolute paths
  • adds deprecations for passing async functions as the Component property

Why is it needed?

  • dependencies up to date!

How to test it?

  • The entire app is effected, the best plan would be a splat attack
  • Try typical use cases, the E2E suite catches barely any of the app although this would have been the best way to ensure everything works.

Related issue(s)/PR(s)

  • resolves CONTENT-1827
  • resolves CONTENT-1759
  • resolves CONTENT-1758
  • resolves CONTENT-1973
  • resolves CONTENT-1757
  • resolves CONTENT-1761
  • resolves CONTENT-1763
  • resolves CONTENT-2127
  • resolves CONTENT-2128
  • resolves CONTENT-2104

@joshuaellis joshuaellis added source: dependencies Source is dependency problem pr: enhancement This PR adds or updates some part of the codebase or features labels Jan 9, 2024
@joshuaellis joshuaellis self-assigned this Jan 9, 2024
@joshuaellis joshuaellis added this to the 5.0.0 milestone Jan 9, 2024
@joshuaellis joshuaellis added the flag: 💥 Breaking change This PR contains breaking changes and should not be merged label Jan 9, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 9, 2024 •
edited

Size Change: +1.75 kB (0%)

Total Size: 1.32 MB

Filename Size Change
examples/getstarted/build/main.********.js 1.31 MB +1.67 kB (0%)
examples/getstarted/build/runtime~main.********.js 4.26 kB +83 B (+2%)
ℹ️ View Unchanged
Filename Size Change
examples/getstarted/build/bb4d0d527bdfb161bc5a.svg 2.33 kB 0 B
examples/getstarted/build/index.html 609 B -1 B (0%)

compressed-size-action

@joshuaellis joshuaellis force-pushed the v5/react-router branch 3 times, most recently from 710bc6f to 1700b6f Compare January 10, 2024 17:35
@joshuaellis joshuaellis marked this pull request as ready for review January 11, 2024 09:24
@joshuaellis joshuaellis force-pushed the v5/react-router branch 2 times, most recently from b0c7d37 to 250f5d2 Compare January 11, 2024 15:58
remidej
remidej reviewed Jan 12, 2024
View reviewed changes
Copy link
Contributor

@remidej remidej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this react router 6 of yours is pretty neat

examples/getstarted/src/plugins/myplugin/admin/src/index.js Outdated Show resolved Hide resolved
...ges/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/Provider.tsx Show resolved Hide resolved
packages/core/admin/admin/src/content-manager/hooks/useLayouts.ts Outdated Show resolved Hide resolved
packages/core/admin/admin/src/content-manager/pages/EditSettingsView/index.jsx Show resolved Hide resolved
packages/core/admin/admin/src/content-manager/pages/EditSettingsView/index.jsx Show resolved Hide resolved
packages/core/content-releases/admin/src/components/CMReleasesContainer.tsx Outdated Show resolved Hide resolved
packages/core/content-type-builder/admin/src/pages/ListView/ListView.tsx
@@ -6,7 +6,7 @@ import has from 'lodash/has';
import isEqual from 'lodash/isEqual';
import upperFirst from 'lodash/upperFirst';
import { useIntl } from 'react-intl';
import { Prompt, useRouteMatch } from 'react-router-dom';
import { unstable_usePrompt as usePrompt, useMatch } from 'react-router-dom';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as earlier about usePrompt

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same response – OOS for the PR, I would talk to DX about the handling the CTB in this case :)

packages/plugins/i18n/admin/src/components/CMEditViewLocalePicker.tsx Show resolved Hide resolved
...ages/plugins/users-permissions/admin/src/pages/Roles/pages/ListPage/components/TableBody.jsx Outdated Show resolved Hide resolved
packages/plugins/users-permissions/admin/src/index.js Outdated Show resolved Hide resolved
remidej
remidej reviewed Jan 15, 2024
View reviewed changes
Copy link
Contributor

@remidej remidej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happy to do qa after we sort these comments :)

packages/core/admin/admin/src/pages/Settings/pages/Webhooks/ListPage.tsx Outdated Show resolved Hide resolved
packages/plugins/i18n/admin/src/components/CMEditViewLocalePicker.tsx Outdated Show resolved Hide resolved
packages/core/admin/admin/src/StrapiApp.tsx Outdated Show resolved Hide resolved
packages/core/admin/admin/src/StrapiApp.tsx Outdated Show resolved Hide resolved
@joshuaellis
Copy link
Member Author

I'll rebase when we're all happy with the changes 👍🏼

remidej
remidej approved these changes Jan 15, 2024
View reviewed changes
Copy link
Contributor

@remidej remidej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I clicked around a bunch of times and couldn't break it 📈

@joshuaellis
feat: update to react-router-dom@6
3e409b3 
chore: refactor all routes to use the children design pattern

chore: convert app to use React.lazy

chore: fix ts errors

test: fix all broken tests from router changes

chore: revert adding `path` property and just append to routes with splat

chore: cleanup deps

chore(releases): convert to rrd@6

chore: fix broken test timeout with EditSettingsView

fix: warnings & add tests around deprecation messages

test(content-releases): fix mocking issue

test(content-manager): fix e2e tests

chore: minor tweaks

chore: rename screaming case to getter

chore: pr fixes & amends
@joshuaellis joshuaellis merged commit cc1043c into v5/main Jan 15, 2024
84 checks passed
@joshuaellis joshuaellis deleted the v5/react-router branch January 15, 2024 12:57
@echoes-hq echoes-hq bot added the echoes/type: maintenance Upkeeping efforts, chores and catch-up corrective improvements that are not features nor bugs label Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@remidej remidej remidej approved these changes

@jhoward1994 jhoward1994 Awaiting requested review from jhoward1994

Assignees

@joshuaellis joshuaellis

Labels
echoes/type: maintenance Upkeeping efforts, chores and catch-up corrective improvements that are not features nor bugs flag: 💥 Breaking change This PR contains breaking changes and should not be merged pr: enhancement This PR adds or updates some part of the codebase or features source: dependencies Source is dependency problem version: 5
Projects
Breaking Change Issues
Status: Done
Milestone
5.0.0-beta.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@joshuaellis @remidej