Enable support for React 18 #763
Conversation
| "@openedx/frontend-build": "14.2.2", | ||
| "@openedx/paragon": "22.13.0", | ||
| "@openedx/frontend-build": "github:adamstankiewicz/frontend-build#ags/react-18", | ||
| "@openedx/paragon": "22.15.1", |
There was a problem hiding this comment.
[inform] Paragon v22 supports React 18! 🎉
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #763 +/- ##
=======================================
Coverage 83.41% 83.41%
=======================================
Files 40 40
Lines 1073 1073
Branches 197 197
=======================================
Hits 895 895
Misses 166 166
Partials 12 12 ☔ View full report in Codecov by Sentry. |
cb4a241 to
8a7ad83
Compare
0f9003e to
5129190
Compare
5129190 to
eecbcf6
Compare
| const DynamicProvider = lazy(() => import('react-redux') | ||
| .then((module) => ({ default: module.Provider })) | ||
| .catch(() => ({ default: ({ children }) => children }))); |
There was a problem hiding this comment.
this is the only part that's making me do a Tim Allen's hruuuh?? 😅
I guess to test my understanding, the OptionalReduxProvider needs to be synchronous with the fetching of a Redux Provider, because react-redux is now explicitly optional in the package.json?
There was a problem hiding this comment.
Before, OptionalReduxProvider assumes react-redux existed, and could be imported statically. With redux and react-redux now marked as optional peer dependencies, the import for react-redux is done at time of render instead of at build.
Now, once OptionalReduxProvider renders, React.lazy will suspend the component (rendering the below Suspense fallback) until the promise resolves. If the module import was successful, React.lazy resolves to the Provider component; otherwise, if the module doesn't exist, the error gets caught within React.lazy whereby the Provider component becomes largely a no-op component that simply renders its children.
There was a problem hiding this comment.
Well put, totally makes sense, thanks! I'll be aiming to test this PR (and frontend-build's) this week!
There was a problem hiding this comment.
LGTM!
I'm currently waiting on a PR sandbox deploy for openedx/frontend-app-authoring#1675. Once that is up and I've done some testing I'll approve openedx/frontend-build#625. Once that lands and this is updated to use the published version of frontend-build I'll approve this one!
| @@ -56,7 +56,7 @@ | |||
| "@cospired/i18n-iso-languages": "4.2.0", | |||
| "@formatjs/intl-pluralrules": "4.3.3", | |||
| "@formatjs/intl-relativetimeformat": "10.0.1", | |||
| "axios": "1.6.7", | |||
| "axios": "1.7.9", | |||
There was a problem hiding this comment.
[inform] resolves a security vulnerability
Description:
Related PRs:
The Open edX MFEs are overdue to support React 18 (especially as React 19 is already out now, too). Some efforts have been started to support React 18 in the shared JavaScript libraries, such as Paragon (openedx/paragon#3367, v22 and v23).
By enabling support for React 18 (should be additive-only), we can further enable incremental migration to React 18 within MFEs. Most of the changes are within the example MFE included in this repository for testing.
The React 18 Upgrade Guide was followed.
Merge checklist:
frontend-platform. This can be done by runningnpm startand opening http://localhost:8080.module.config.jsfile infrontend-build.fix,feat) and is appropriate for your code change. Consider whether your code is a breaking change, and modify your commit accordingly.Post merge: