-
-
Notifications
You must be signed in to change notification settings - Fork 184
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 to React 18 #4992
Upgrade to React 18 #4992
Conversation
(Also update webpack-dev-server and autoprefixer while we're here) This commit does not fully build yet; need: - Fix emerging TS errors, from types updates - Make sure react-refresh, etc. works - Make sure all react-based libraries work - See about this EBADENGINE >=16.17.0 in Storybook's dependencies - …and more. Note the react-table override: v6 -> v7 or v8 is a substantial rewrite with no upgrade guide. My hope is that, as with reflux, it was forwards- compatible with react v18, and we can upgrade it later. A variety of deprecation warnings appear when you do a fresh install without package-lock.json. Here's how to view them again: rm -rf ./node_modules npm cache clean -f # superstitiously, to get resolved/integrity * npm install * (https://stackoverflow.com/a/75236714)
If you click the 'change email' button without filling the email input, in prod it appears to do nothing; in dev there's a fullscreen error. We should probably show a toast in case of API error, but meanwhile I through in some light validation to disable the button until there's at least something in the text input.
- https://github.com/gaearon/react-hot-loader?tab=readme-ov-file#moving-towards-next-step - https://github.com/pmmmwh/react-refresh-webpack-plugin - using `swc-loader` > swc-loader will set the development option based on Webpack's mode > option. swc won't enable fast refresh when development is false.
- https://legacy.reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html - https://swc.rs/docs/configuration/compilation#jsctransformreactruntime > "Use `runtime: automatic` to use a JSX runtime module (e.g. `react/jsx-runtime` introduced in React 17)." (Note that with the new jsx transform, you can now use jsx/tsx in a file without importing React. TS knows this now, too. Feel free to take its suggestion.) (The $schema key added to .swcrc just adds better hints when editing the JSON config, it's unrelated to the runtime change.)
0ba49c3
to
3631296
Compare
}, | ||
"react-table": { | ||
"react": "$react", | ||
"react-dom": "$react-dom" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
React Table v6 is very different from v7 or v8, and would be a non-trivial, unguided migration. Staying on v6 for now, we count on the backwards-compatibility of React (and the forwards-compatibility of React Table v6), as we did with reflux.
See:
- Last README entry for react-table v6, v6->v7
The differences between the 2 versions are incredibly massive. Unfortunately, I cannot write a one-to-one upgrade guide for any of v6's API, simply because much of it is irrelevant with v7's headless approach. The best approach for migrating to v7 is to learn its API by reading the documentation and then following some of the examples to begin building your own table component.
- v8 migration guide, v7 -> v8
Pre-emptively install react-query, to make branch switching easier for upcoming PR reviews.
@@ -11,9 +11,11 @@ | |||
"@fontsource/roboto-mono": "^4.4.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need to update the engines
? When I am npm install
ing this branch, I see a warning that I should use node v16.17.0
(it comes with [email protected]
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrading Node will mean updating some Dockerfiles, scripts, and the CI, and will be a bit tricky (even if it's just a point upgrade). I want to do it in another PR.
The 16.17 warning is safe to ignore; it comes from a Storybook CLI sub-sub-dependency. The Storybook dev workflow is unaffected:
- We don't use the feature that Storybook brought the package in for
- The package brings in another package for a feature that Storybook doesn't use either
- The sub-sub-sub dependency probably would work fine with Node 16.15 anyway, but asks for >=16.17 for reasons unspecified by the author
…so our case that code never executes. I'll push a commit that pins a version so we don't have to see EBADENGINE
:
"storybook": {
"giget": "1.1.3" // Keeps execa <8 for unused feature
}
notes from researching this
These are the packages:
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE package: '[email protected]',
npm WARN EBADENGINE required: { node: '>=16.17' },
npm WARN EBADENGINE current: { node: 'v16.15.0', npm: '8.5.5' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE package: '[email protected]',
npm WARN EBADENGINE required: { node: '>=16.17.0' },
npm WARN EBADENGINE current: { node: 'v16.15.0', npm: '8.5.5' }
npm WARN EBADENGINE }
Here's how we end up with it:
storybook cli sandbox feature (that we don't use)
|
v
giget deps feature (that storybook doesn't use)
|
v
nypm (gets updated by a bot)
|
v
execa -> human-signals (stricter versioning than required)
- storybook cli switches from degit to giget ^1 for a feature that we don't use — Nov 18, 2022
- giget 1.2 adds nypm for an optional feature. storybook's feature that we don't use doesn't use that feature. — Dec 23, 2023
- Meanwhile:
- execa 8 (and human-signals) have dropped Node 14, and in the process, specified a stricter Node 16 version range (probably stricter than they need) — Aug 5, 2023
- nypm 0.3.1 updates execa from ^7.2.0 to 8.0.1 in a bot commit just to keep on top of things. — Aug 24, 2023
- Sometime later:
- We update our dependencies. Since Storybook specifies giget ^1, it retrieves the latest version of giget, so we get the dependency warning coming from
giget -> nypm -> execa -> human-signals
.
- We update our dependencies. Since Storybook specifies giget ^1, it retrieves the latest version of giget, so we get the dependency warning coming from
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the research! Looks fine to me :)
commit 8d3d056 Author: Leszek <[email protected]> Date: Fri Jul 12 12:34:21 2024 +0200 small code cleanup in submission modal commit 347684c Author: Leszek <[email protected]> Date: Fri Jul 12 12:33:32 2024 +0200 fix submission modal not loading submission from next page commit d4d2a00 Author: Leszek <[email protected]> Date: Thu Jul 11 10:19:14 2024 +0200 rename few properties for better clarity commit c65f33d Author: Leszek <[email protected]> Date: Wed Jul 10 00:32:16 2024 +0200 cleanup and comment Submission Modal code also cleanup duplicated submission flow a bit commit 8eaae02 Author: Leszek <[email protected]> Date: Tue Jul 9 15:46:54 2024 +0200 use classnames util in KoboModal commit 417d66f Author: Leszek <[email protected]> Date: Tue Jul 9 15:46:38 2024 +0200 small cleanup of KoboSelect styles commit 0d9546c Author: Leszek <[email protected]> Date: Tue Jul 9 15:46:17 2024 +0200 Move background audio handling into Submission Modal from Data Table to make it more independent commit d85bd1f Author: Leszek <[email protected]> Date: Tue Jul 9 15:33:31 2024 +0200 remove unused types import commit 457c564 Author: Leszek <[email protected]> Date: Tue Jul 9 15:33:13 2024 +0200 KoboDropdown name prop is not optional anymore commit a7b1839 Merge: bac5130 368e755 Author: RuthShryock <[email protected]> Date: Mon Jul 8 11:07:11 2024 -0400 Merge pull request #4994 from kobotoolbox/nlp-usage-limit-block-frontend [TASK-461] Add modal(s) to NLP page when user is over limits commit bac5130 Merge: a8e4243 0e21155 Author: Philip Edwards <[email protected]> Date: Thu Jul 4 11:48:09 2024 -0400 Merge pull request #4992 from kobotoolbox/react-18-upgrade Upgrade to React 18 commit 0e21155 Author: Phil Edwards <[email protected]> Date: Thu Jul 4 11:03:19 2024 -0400 Pin unused storybook dependency, removes EBADENGINE warning #4992 (comment) commit a8e4243 Merge: fac7edf 71920e9 Author: Olivier Léger <[email protected]> Date: Thu Jul 4 09:50:12 2024 -0400 Merge pull request #4988 from kobotoolbox/remove-constance-values-assigned-in-tests Refactor `constance` setting assignments in tests to use `override_config` commit 7d3da93 Author: Leszek <[email protected]> Date: Thu Jul 4 14:13:51 2024 +0200 fix edit library item button link commit 4b844ae Author: Leszek <[email protected]> Date: Thu Jul 4 13:59:10 2024 +0200 fix _prepColumns on validation status change commit d1eec97 Author: Leszek <[email protected]> Date: Thu Jul 4 13:55:31 2024 +0200 fix _prepColumns on initial Data Table load commit 368e755 Author: James Kiger <[email protected]> Date: Wed Jul 3 13:12:52 2024 -0400 Remove extra button styling commit fac7edf Merge: 167f665 c234e4e Author: Philip Edwards <[email protected]> Date: Wed Jul 3 12:42:11 2024 -0400 Merge pull request #4991 from kobotoolbox/organizations-error-anon-user Retrieve organization data only for logged-in users to prevent error messages commit 71920e9 Author: RuthShryock <[email protected]> Date: Wed Jul 3 10:45:12 2024 -0400 keep direct imports at the top commit a2355d4 Author: RuthShryock <[email protected]> Date: Wed Jul 3 09:57:40 2024 -0400 fix formatting and use LazyJSONSerializable instead of json.dumps commit 8396b20 Author: Phil Edwards <[email protected]> Date: Tue Jul 2 16:03:02 2024 -0400 Remove bonus console.log commit 3cfebca Author: Phil Edwards <[email protected]> Date: Tue Jul 2 13:08:44 2024 -0400 Pass tests that would fail locally after mocha / chai / deep-eql update commit 28eb4cb Author: Phil Edwards <[email protected]> Date: Tue Jul 2 11:21:06 2024 -0400 Install react-query v5 Pre-emptively install react-query, to make branch switching easier for upcoming PR reviews. commit 3631296 Author: Phil Edwards <[email protected]> Date: Mon Jul 1 17:36:55 2024 -0400 Use React's "automatic" runtime instead of "classic" - https://legacy.reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html - https://swc.rs/docs/configuration/compilation#jsctransformreactruntime > "Use `runtime: automatic` to use a JSX runtime module (e.g. `react/jsx-runtime` introduced in React 17)." (Note that with the new jsx transform, you can now use jsx/tsx in a file without importing React. TS knows this now, too. Feel free to take its suggestion.) (The $schema key added to .swcrc just adds better hints when editing the JSON config, it's unrelated to the runtime change.) commit df1d722 Author: Phil Edwards <[email protected]> Date: Mon Jul 1 17:24:11 2024 -0400 Remove react-hot-loader in favor of react-refresh - https://github.com/gaearon/react-hot-loader?tab=readme-ov-file#moving-towards-next-step - https://github.com/pmmmwh/react-refresh-webpack-plugin - using `swc-loader` > swc-loader will set the development option based on Webpack's mode > option. swc won't enable fast refresh when development is false. commit f49b046 Author: Phil Edwards <[email protected]> Date: Mon Jul 1 17:15:42 2024 -0400 Bump several build-related packages commit 10bb3c0 Author: Phil Edwards <[email protected]> Date: Mon Jul 1 12:11:13 2024 -0400 Validate 'change email' field If you click the 'change email' button without filling the email input, in prod it appears to do nothing; in dev there's a fullscreen error. We should probably show a toast in case of API error, but meanwhile I through in some light validation to disable the button until there's at least something in the text input. commit 30862c5 Author: Phil Edwards <[email protected]> Date: Fri Jun 28 14:43:46 2024 -0400 Opt-in to the React 18 root API https://react.dev/blog/2022/03/08/react-18-upgrade-guide#updates-to-client-rendering-apis commit a008ee3 Author: James Kiger <[email protected]> Date: Tue Jul 2 08:20:34 2024 -0400 Use enum for usage limit types commit 2b4ac08 Author: James Kiger <[email protected]> Date: Tue Jul 2 07:08:40 2024 -0400 Minor corrections commit c234e4e Author: RuthShryock <[email protected]> Date: Mon Jul 1 15:28:03 2024 -0400 don't get organization data if the user is not logged in commit 66c8b59 Author: Phil Edwards <[email protected]> Date: Fri Jun 28 13:39:48 2024 -0400 Satisfy updated TS / react types. commit 5c7a8d1 Author: Phil Edwards <[email protected]> Date: Thu Jun 27 18:15:20 2024 -0400 Update react to v18, allowing semver updates (Also update webpack-dev-server and autoprefixer while we're here) This commit does not fully build yet; need: - Fix emerging TS errors, from types updates - Make sure react-refresh, etc. works - Make sure all react-based libraries work - See about this EBADENGINE >=16.17.0 in Storybook's dependencies - …and more. Note the react-table override: v6 -> v7 or v8 is a substantial rewrite with no upgrade guide. My hope is that, as with reflux, it was forwards- compatible with react v18, and we can upgrade it later. A variety of deprecation warnings appear when you do a fresh install without package-lock.json. Here's how to view them again: rm -rf ./node_modules npm cache clean -f # superstitiously, to get resolved/integrity * npm install * (https://stackoverflow.com/a/75236714) commit d47f8cc Author: James Kiger <[email protected]> Date: Mon Jul 1 10:51:17 2024 -0400 Styling commit cfb38da Author: James Kiger <[email protected]> Date: Mon Jul 1 08:48:09 2024 -0400 Clean up handling of seconds-to-minutes conversion for usage/limits commit d0305ba Author: RuthShryock <[email protected]> Date: Fri Jun 28 17:23:36 2024 -0400 use override_config in test cases where config values are directly assigned commit 039566d Author: James Kiger <[email protected]> Date: Fri Jun 28 13:47:54 2024 -0400 Add modal for blocking frontend nlp usage
Overview
This branch:
react-hot-loader
in favor of react-refreshOverall, pretty smooth.
EBADENGINE
warning from its CLI (which will go away once we update Node)To keep things moving along, I haven't migrated every deprecated React function in this PR. (Particularly if it comes from a legacy class that might be rewritten soon). Some things that still work in React 18 (but will be removed in React 19) can be found in the console after a refresh. Examples:
contextTypes
)ref
property may return a component instance instead of a DOM node, and that led to a problem inkoboDropdown
when I tried replacing this found node with aref.current
.)Related issues
Part of #4703
Description
Update React and related packages.
Checklist