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.

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

Upgrade react router #782

Merged
merged 8 commits into from
Jan 27, 2025
Merged

Conversation

sergei-maertens
Copy link
Member

Copy link

codecov bot commented Jan 22, 2025

Bundle Report

Changes will increase total bundle size by 38.56kB (0.41%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
@open-formulieren/sdk-OpenForms-umd 4.77MB 19.29kB (0.41%) ⬆️
@open-formulieren/sdk-esm 4.77MB 19.27kB (0.41%) ⬆️

Copy link

codecov bot commented Jan 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.41%. Comparing base (49bd7cb) to head (12e2591).
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #782      +/-   ##
==========================================
+ Coverage   83.32%   83.41%   +0.09%     
==========================================
  Files         239      240       +1     
  Lines        4750     4752       +2     
  Branches     1268     1275       +7     
==========================================
+ Hits         3958     3964       +6     
+ Misses        757      754       -3     
+ Partials       35       34       -1     
Flag Coverage Δ
storybook 75.83% <100.00%> (+0.05%) ⬆️
vitest 62.76% <100.00%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sergei-maertens sergei-maertens force-pushed the issue/4929-bump-to-latest-react-router-v6 branch from 293f6fb to a9f126d Compare January 23, 2025 14:24
@sergei-maertens sergei-maertens marked this pull request as ready for review January 23, 2025 15:03
@sergei-maertens sergei-maertens force-pushed the issue/4929-bump-to-latest-react-router-v6 branch from a9f126d to b05c731 Compare January 23, 2025 15:04
…aviour

Following the upgrade instructions of the official
documentation.
These are for features that we don't make use of yet,
following the docs to just enable them and rely on our
test suites.
* Removed the future flags again
* Replaced the react-router-dom package with react-router
This should probably come via our shared Maykin config...

In essence, import-js/eslint-plugin-import#2862
describes the problem that the typescript-eslint-plugin parser is not
hoisted to the top-level node modules, and it's being dynamically
imported during linting of src/sdk.jsx which loads react-router/dom,
which has typescript declaration files. Adding this explicit
dependency makes the linter error go away.
In vitest the react-router and react-router/dom imports resolve to
(CommonJS?) bundles which each contain copies of the contexts/objects
used by react router, which make you effectively end up with multiple
installations of react router. This leads to broken contexts and failing
tests, see remix-run/react-router#12785 for
more context.

Patching the resolution in vite config allows us to force loading the
mjs modules instead of the CJS build which doesn't have this problem,
because both the react-router and react-router/dom entrypoints depend
on the same chunk containing the shared code.
@sergei-maertens sergei-maertens force-pushed the issue/4929-bump-to-latest-react-router-v6 branch from b05c731 to 12e2591 Compare January 23, 2025 15:07
Copy link
Contributor

@robinmolen robinmolen left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@robinmolen robinmolen merged commit 385816d into main Jan 27, 2025
17 checks passed
@robinmolen robinmolen deleted the issue/4929-bump-to-latest-react-router-v6 branch January 27, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to React Router v7
2 participants