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

FED-3273 SPIKE - do not merge: React 18 dual versions #82

Draft
wants to merge 9 commits into
base: r18
Choose a base branch
from

Conversation

greglittlefield-wf
Copy link
Contributor

@greglittlefield-wf greglittlefield-wf commented Feb 11, 2025

Motivation

We wanted to spike out updating the existing RTL JS assets be compatible with both React 17 and 18, to make the migration to React 18 easier.

With that in place, we wouldn't have to worry about dual-JS files or separate release lines, or coordinating them with the react-dart version used when testing.

This is just a spike branch; not for merging.

Solution

  • Spike out solution that bundles both 17- and 18-compatible versions of RTL into the existing JS file, and loads the correct one conditionally based on window.React.version
  • Add test to validate current React version
  • Add example used to do help debug during development

Testing steps

  • Verify React 18 branch this PR is based on has passing CI: React 18 #81
  • Verify CI passes and react_version_test.dart test run

Derived from building master (3.0.2) with the following changes:

    diff --git a/js_src/rollup.config.js b/js_src/rollup.config.js
    index 38448f5d..e8432dfc 100644
    --- a/js_src/rollup.config.js
    +++ b/js_src/rollup.config.js
    @@ -68 +68 @@ export default (commandFlags) => {
    -        format: 'umd',
    +        format: 'esm',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From 94435ac

Add ESM version of current React-17-compatible RTL bundle

Derived from building master (3.0.2) with the following changes:

diff --git a/js_src/rollup.config.js b/js_src/rollup.config.js
index 38448f5d..e8432dfc 100644
--- a/js_src/rollup.config.js
+++ b/js_src/rollup.config.js
@@ -68 +68 @@ export default (commandFlags) => {
-        format: 'umd',
+        format: 'esm',

I added this to prove out pulling in both, as opposed to updating this NPM package to pull in and build two versions of the same package. For the real implementation we could try to do that, or just stick with this since we probably won't need to update the 17-compatible assets until they're eventually removed.

I originally tried with the UMD, but it was kind of gross since it assigned to the global, so I went with an ESM build to make the bundling simpler/cleaner.

@greglittlefield-wf greglittlefield-wf changed the title React 18 dual version spike FED-3273 SPIKE - do not merge: React 18 dual versions Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants