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

Throw if React and React DOM versions don't match #29236

Merged
merged 2 commits into from
May 28, 2024

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented May 23, 2024

Throw an error during module initialization if the version of the "react-dom" package does not match the version of "react".

We used to be more relaxed about this, because the "react" package changed so infrequently. However, we now have many more features that rely on an internal protocol between the two packages, including Hooks, Float, and the compiler runtime. So it's important that both packages are versioned in lockstep.

Before this change, a version mismatch would often result in a cryptic internal error with no indication of the root cause.

Instead, we will now compare the versions during module initialization and immediately throw an error to catch mistakes as early as possible and provide a clear error message.

Copy link

vercel bot commented May 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 28, 2024 5:59pm

@react-sizebot
Copy link

react-sizebot commented May 23, 2024

Comparing: 4ec6a6f...38bc3c5

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.66 kB 6.66 kB +0.05% 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js +0.06% 496.04 kB 496.36 kB +0.07% 88.77 kB 88.83 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js +0.07% 500.84 kB 501.18 kB +0.08% 89.46 kB 89.52 kB
facebook-www/ReactDOM-prod.classic.js +0.05% 593.48 kB 593.80 kB +0.06% 104.38 kB 104.44 kB
facebook-www/ReactDOM-prod.modern.js +0.05% 569.87 kB 570.18 kB +0.08% 100.77 kB 100.85 kB
test_utils/ReactAllWarnings.js Deleted 63.82 kB 0.00 kB Deleted 15.95 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-dom/cjs/react-dom-server.node.production.js +0.25% 237.81 kB 238.41 kB +0.49% 42.61 kB 42.82 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.production.js +0.25% 241.44 kB 242.04 kB +0.47% 43.26 kB 43.46 kB
oss-stable/react-dom/cjs/react-dom-server.bun.production.js +0.24% 200.08 kB 200.57 kB +0.58% 37.21 kB 37.42 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.production.js +0.24% 216.21 kB 216.72 kB +0.50% 39.35 kB 39.54 kB
oss-stable/react-dom/cjs/react-dom-server.node.production.js +0.23% 212.73 kB 213.22 kB +0.52% 39.05 kB 39.26 kB
oss-stable/react-dom/cjs/react-dom-server.edge.production.js +0.23% 216.52 kB 217.01 kB +0.48% 40.31 kB 40.51 kB
oss-stable-semver/react-dom/cjs/react-dom-server.bun.production.js +0.22% 200.06 kB 200.50 kB +0.58% 37.19 kB 37.40 kB
oss-stable-semver/react-dom/cjs/react-dom-server.node.production.js +0.21% 212.70 kB 213.15 kB +0.52% 39.03 kB 39.23 kB
oss-stable-semver/react-dom/cjs/react-dom-server.edge.production.js +0.21% 216.50 kB 216.94 kB +0.48% 40.28 kB 40.48 kB
test_utils/ReactAllWarnings.js Deleted 63.82 kB 0.00 kB Deleted 15.95 kB 0.00 kB

Generated by 🚫 dangerJS against 38bc3c5

if (isomorphicReactPackageVersion !== reactDOMPackageVersion) {
// TODO: Do we have a canoncical documentation page for this?
throw new Error(
'Incompatible React versions: The "react" and "react-dom" packages must ' +
Copy link
Collaborator

Choose a reason for hiding this comment

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

In a perfect world we'd also detect environment mismatches e.g. importing react-dom with the react-server condition while react is being imported without the react-server condition. Maybe we encode the environment in the version string e.g. 19.0.0-abc-123+react-server? Though that wouldn't tell you explcitly that the environment mismatched not the version.

Copy link
Collaborator Author

@acdlite acdlite May 23, 2024

Choose a reason for hiding this comment

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

We could add another internal field (or check for the presence of a server-only API, and vice versa), but I'm less concerned about that one since it's only the tip of the iceberg of what you have to do to configure a Server Components set-up correctly. For a similar reason I didn't add a check to React Native because nobody really imports the React Native renderer directly; it's configured by some sort of framework.

Copy link
Member

Choose a reason for hiding this comment

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

Can we actually add it for React Native too? Too often the RN sync are using the wrong versions together, which caused issues when landing breaking changes

Copy link
Member

Choose a reason for hiding this comment

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

Like, RN installs the renderer, but the template still allows using your own version of react, and we should catch that. This will start erroring in the OSS build on land, but that needs to get fixed before the next RN npm release.

@acdlite acdlite force-pushed the throw-if-react-version-mismatch branch from d7c40ca to 8ad45be Compare May 23, 2024 23:48
@acdlite acdlite marked this pull request as ready for review May 23, 2024 23:59
export function ensureCorrectIsomorphicReactVersion() {
const isomorphicReactPackageVersion = IsomorphicReactPackage.version;
if (isomorphicReactPackageVersion !== reactDOMPackageVersion) {
// TODO: Do we have a canoncical documentation page for this?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Closest is https://legacy.reactjs.org/warnings/invalid-hook-call-warning.html#mismatching-versions-of-react-and-react-dom and https://react.dev/warnings/invalid-hook-call-warning#mismatching-versions-of-react-and-react-dom. But these are just about a React version that's too low or duplicate React modules not explicitly about version mismatch. For example, package managers that auto-install peer dependencies will result in multiple versions of React if the specified version of react and react-dom don't match.

So maybe we include version mismatch in that section and link to it?

Copy link
Member

@rickhanlonii rickhanlonii May 24, 2024

Choose a reason for hiding this comment

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

Just added react.dev/warnings/version-mismatch, @acdlite can we add that URL to the error? reactjs/react.dev#6909

Copy link
Member

Choose a reason for hiding this comment

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

It just redirects for now, but I'll follow up with a proper warning page

@eps1lon
Copy link
Collaborator

eps1lon commented May 24, 2024

react-is is another candidate for this. ReactIs.isElement will also not work if you use React 19 with an earlier react-is (like recharts did) or vice versa. The fix is usually to move react-is to peer dependencies so that the app can ensure the version matches.

@acdlite
Copy link
Collaborator Author

acdlite commented May 24, 2024

Weren't we going to delete react-is entirely though? :D

@eps1lon
Copy link
Collaborator

eps1lon commented May 24, 2024

Some design system libraries need at least support for isElement (can be moved to react) and isValidElementType (should be moved to renderers). You oftentimes find APIs were either element types or elements are expected in a prop and these utils help identify what you actually received. Though isElement only makes sense in the context of element introspection (e.g. element.props) or cloneElement. If we remove those APIs, I can't think of use cases for isElement apart from runtime type checking. Still leaves isValidElementType.

@rickhanlonii
Copy link
Member

React has isValidElement already? https://react.dev/reference/react/isValidElement

@unstubbable
Copy link
Contributor

unstubbable commented May 24, 2024

Another prominent use case is Jest snapshot testing, as reported in #28813 (comment). A version mismatch might break those tests because they rely on react-is.

@eps1lon
Copy link
Collaborator

eps1lon commented May 24, 2024

Another prominent use case is Jest snapshot testing, as reported in #28813 (comment). A version mismatch might break those because they rely on react-is.

I think what pretty-format needs is an API to get the display name not necessarily an API to change control flow based of the element kind. So for them it seems sufficient if we'd start implementing .displayName on every element type.

*/

import reactDOMPackageVersion from 'shared/ReactVersion';
import * as IsomorphicReactPackage from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we compile to CJS this doesn't matter much but it might be nice to import {version} from "react" so that it doesn't pull in every export and disables dead export elimination.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've been doing it this way based on this comment. Not sure if it still applies:

// This module only exists as an ESM wrapper around the external CommonJS
// Scheduler dependency. Notice that we're intentionally not using named imports
// because Rollup would use dynamic dispatch for CommonJS interop named imports.
// When we switch to ESM, we can delete this module.
import * as Scheduler from 'scheduler';

Throw an error during module initialization if the version of the
"react-dom" package does not match the version of "react".

We used to be more relaxed about this, because the "react" package
changed so infrequently. However, we now have many more features that
rely on an internal protocol between the two packages, including Hooks,
Float, and the compiler runtime. So it's important that both packages
are versioned in lockstep.

Before this change, a version mismatch would often result in a cryptic
internal error with no indication of the root cause.

Instead, we will now compare the versions during module initialization
and immediately throw an error to catch mistakes as early as possible
and provide a clear error message.
@acdlite acdlite force-pushed the throw-if-react-version-mismatch branch from e8b016c to 38bc3c5 Compare May 28, 2024 17:56
@acdlite acdlite merged commit 681a4aa into facebook:main May 28, 2024
40 checks passed
github-actions bot pushed a commit that referenced this pull request May 28, 2024
Throw an error during module initialization if the version of the
"react-dom" package does not match the version of "react".

We used to be more relaxed about this, because the "react" package
changed so infrequently. However, we now have many more features that
rely on an internal protocol between the two packages, including Hooks,
Float, and the compiler runtime. So it's important that both packages
are versioned in lockstep.

Before this change, a version mismatch would often result in a cryptic
internal error with no indication of the root cause.

Instead, we will now compare the versions during module initialization
and immediately throw an error to catch mistakes as early as possible
and provide a clear error message.

DiffTrain build for commit 681a4aa.
github-actions bot pushed a commit that referenced this pull request May 28, 2024
Throw an error during module initialization if the version of the
"react-dom" package does not match the version of "react".

We used to be more relaxed about this, because the "react" package
changed so infrequently. However, we now have many more features that
rely on an internal protocol between the two packages, including Hooks,
Float, and the compiler runtime. So it's important that both packages
are versioned in lockstep.

Before this change, a version mismatch would often result in a cryptic
internal error with no indication of the root cause.

Instead, we will now compare the versions during module initialization
and immediately throw an error to catch mistakes as early as possible
and provide a clear error message.

DiffTrain build for [681a4aa](681a4aa)
nikeee pushed a commit to nikeee/react that referenced this pull request May 28, 2024
Throw an error during module initialization if the version of the
"react-dom" package does not match the version of "react".

We used to be more relaxed about this, because the "react" package
changed so infrequently. However, we now have many more features that
rely on an internal protocol between the two packages, including Hooks,
Float, and the compiler runtime. So it's important that both packages
are versioned in lockstep.

Before this change, a version mismatch would often result in a cryptic
internal error with no indication of the root cause.

Instead, we will now compare the versions during module initialization
and immediately throw an error to catch mistakes as early as possible
and provide a clear error message.
yungsters added a commit that referenced this pull request May 30, 2024
#29236 caused issues for internal
syncs at Meta, because we were computing version numbers using file
hashes (to eliminate "no-op" internal sync commits). The problem is that
since version numbers may not be consistent across synced files (e.g. if
some files have not changed in recent commits), the newly introduced
version mismatch check fails.

There's some more work that needs to be done here to restore the
benefits of file-specific hashing, but for now this simply reverts the
content hash changes from the following PRs:

- #28633
(95319ab)
- #28590
(37676ab)
- #28582
(cb076b5)
- #26734
(5dd90c5)
- #26331
(3cad3a5)
github-actions bot pushed a commit that referenced this pull request May 30, 2024
#29236 caused issues for internal
syncs at Meta, because we were computing version numbers using file
hashes (to eliminate "no-op" internal sync commits). The problem is that
since version numbers may not be consistent across synced files (e.g. if
some files have not changed in recent commits), the newly introduced
version mismatch check fails.

There's some more work that needs to be done here to restore the
benefits of file-specific hashing, but for now this simply reverts the
content hash changes from the following PRs:

- #28633
(95319ab)
- #28590
(37676ab)
- #28582
(cb076b5)
- #26734
(5dd90c5)
- #26331
(3cad3a5)

DiffTrain build for [5bd4031](5bd4031)
github-actions bot pushed a commit that referenced this pull request May 30, 2024
#29236 caused issues for internal
syncs at Meta, because we were computing version numbers using file
hashes (to eliminate "no-op" internal sync commits). The problem is that
since version numbers may not be consistent across synced files (e.g. if
some files have not changed in recent commits), the newly introduced
version mismatch check fails.

There's some more work that needs to be done here to restore the
benefits of file-specific hashing, but for now this simply reverts the
content hash changes from the following PRs:

- #28633
(95319ab)
- #28590
(37676ab)
- #28582
(cb076b5)
- #26734
(5dd90c5)
- #26331
(3cad3a5)

DiffTrain build for commit 5bd4031.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants