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

fix: add react-server condition for react/jsx-dev-runtime #28921

Merged
merged 5 commits into from Apr 27, 2024

Conversation

himself65
Copy link
Contributor

@himself65 himself65 commented Apr 26, 2024

Summary

Add react-server condition for react/jsx-dev-runtime to ensure they use the same internal in one node thread.

In Waku RSC, we have a node worker under the react-server condition and use vite to parse the react component. in development mode, the jsx-dev was linked to React.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE, which does not exist on the server side. So react@beta cause a runtime error and blocks waku from upgrading react version(dai-shi/waku#671).

Cannot process SSR Error: TypeError: Cannot read properties of undefined (reading 'A')
    at file:///home/runner/work/waku/waku/packages/waku/dist/lib/renderers/dev-worker-api.js:132:60
    at Worker.<anonymous> (file:///home/runner/work/waku/waku/packages/waku/dist/lib/renderers/dev-worker-api.js:34:52)
    at Worker.emit (node:events:526:35)
    at MessagePort.<anonymous> (node:internal/worker:263:53)
    at [nodejs.internal.kHybridDispatch] (node:internal/event_target:807:20)
    at exports.emitMessage (node:internal/per_context/messageport:23:28)

Related: #28217

/cc @dai-shi

How did you test this change?

Copy and paste into waku repo node_modules; not sure how to test React.

@react-sizebot
Copy link

react-sizebot commented Apr 26, 2024

Comparing: 4ddff73...faf1bc6

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 = 1.82 kB 1.82 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB = 1.83 kB 1.83 kB
facebook-www/ReactDOM-prod.classic.js = 591.11 kB 591.11 kB = 103.94 kB 103.94 kB
facebook-www/ReactDOM-prod.modern.js = 567.33 kB 567.33 kB = 100.34 kB 100.34 kB
oss-experimental/react/cjs/react-jsx-dev-runtime.react-server.development.js +∞% 0.00 kB 43.05 kB +∞% 0.00 kB 12.99 kB
oss-experimental/react/cjs/react-jsx-dev-runtime.react-server.production.js +∞% 0.00 kB 1.32 kB +∞% 0.00 kB 0.68 kB
oss-experimental/react/jsx-dev-runtime.react-server.js +∞% 0.00 kB 0.25 kB +∞% 0.00 kB 0.16 kB
oss-stable-semver/react/cjs/react-jsx-dev-runtime.react-server.development.js +∞% 0.00 kB 43.05 kB +∞% 0.00 kB 12.99 kB
oss-stable-semver/react/cjs/react-jsx-dev-runtime.react-server.production.js +∞% 0.00 kB 1.32 kB +∞% 0.00 kB 0.68 kB
oss-stable-semver/react/jsx-dev-runtime.react-server.js +∞% 0.00 kB 0.25 kB +∞% 0.00 kB 0.16 kB
oss-stable/react/cjs/react-jsx-dev-runtime.react-server.development.js +∞% 0.00 kB 43.05 kB +∞% 0.00 kB 12.99 kB
oss-stable/react/cjs/react-jsx-dev-runtime.react-server.production.js +∞% 0.00 kB 1.32 kB +∞% 0.00 kB 0.68 kB
oss-stable/react/jsx-dev-runtime.react-server.js +∞% 0.00 kB 0.25 kB +∞% 0.00 kB 0.16 kB
test_utils/ReactAllWarnings.js Deleted 64.26 kB 0.00 kB Deleted 16.02 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/cjs/react-jsx-dev-runtime.react-server.development.js +∞% 0.00 kB 43.05 kB +∞% 0.00 kB 12.99 kB
oss-experimental/react/cjs/react-jsx-dev-runtime.react-server.production.js +∞% 0.00 kB 1.32 kB +∞% 0.00 kB 0.68 kB
oss-experimental/react/jsx-dev-runtime.react-server.js +∞% 0.00 kB 0.25 kB +∞% 0.00 kB 0.16 kB
oss-stable-semver/react/cjs/react-jsx-dev-runtime.react-server.development.js +∞% 0.00 kB 43.05 kB +∞% 0.00 kB 12.99 kB
oss-stable-semver/react/cjs/react-jsx-dev-runtime.react-server.production.js +∞% 0.00 kB 1.32 kB +∞% 0.00 kB 0.68 kB
oss-stable-semver/react/jsx-dev-runtime.react-server.js +∞% 0.00 kB 0.25 kB +∞% 0.00 kB 0.16 kB
oss-stable/react/cjs/react-jsx-dev-runtime.react-server.development.js +∞% 0.00 kB 43.05 kB +∞% 0.00 kB 12.99 kB
oss-stable/react/cjs/react-jsx-dev-runtime.react-server.production.js +∞% 0.00 kB 1.32 kB +∞% 0.00 kB 0.68 kB
oss-stable/react/jsx-dev-runtime.react-server.js +∞% 0.00 kB 0.25 kB +∞% 0.00 kB 0.16 kB
test_utils/ReactAllWarnings.js Deleted 64.26 kB 0.00 kB Deleted 16.02 kB 0.00 kB

Generated by 🚫 dangerJS against faf1bc6

@himself65 himself65 changed the title fix: react server condition for react/jsx-dev-runtime fix: add react-server condition for react/jsx-dev-runtime Apr 26, 2024
eps1lon added a commit to vercel/next.js that referenced this pull request Apr 27, 2024
eps1lon added a commit to vercel/next.js that referenced this pull request Apr 27, 2024
eps1lon added a commit to vercel/next.js that referenced this pull request Apr 27, 2024
Copy link
Collaborator

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Thank you!

@eps1lon eps1lon merged commit 8090457 into facebook:main Apr 27, 2024
38 checks passed
'use strict';

if (process.env.NODE_ENV === 'production') {
module.exports = require('./cjs/react-jsx-dev-runtime.react-server.production.min.js');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just spotted this. We no longer ship minified so I'll need to follow-up on this one: #28939

@himself65 himself65 deleted the himself65/react-jsx-dev branch April 27, 2024 20:18
eps1lon added a commit to vercel/next.js that referenced this pull request Apr 28, 2024
eps1lon added a commit to vercel/next.js that referenced this pull request Apr 29, 2024
eps1lon added a commit to vercel/next.js that referenced this pull request Apr 29, 2024
eps1lon added a commit to vercel/next.js that referenced this pull request Apr 30, 2024
dai-shi added a commit to dai-shi/waku that referenced this pull request May 1, 2024
Closes: #671
Upstream: facebook/react#28921

---------

Co-authored-by: daishi <daishi@axlight.com>
eps1lon added a commit to vercel/next.js that referenced this pull request May 2, 2024
eps1lon added a commit to vercel/next.js that referenced this pull request May 2, 2024
eps1lon added a commit to vercel/next.js that referenced this pull request May 3, 2024
eps1lon added a commit to vercel/next.js that referenced this pull request May 6, 2024
eps1lon added a commit to vercel/next.js that referenced this pull request May 6, 2024
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.

None yet

4 participants