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

[Flight] provide property descriptors for client references #27328

Merged
merged 1 commit into from
Sep 5, 2023

Conversation

gnoff
Copy link
Collaborator

@gnoff gnoff commented Sep 2, 2023

Client reference proxy should implement getOwnPropertyDescriptor. One practical place where this shows up is when consuming CJS module.exports in ESM modules. Node creates named exports it statically infers from the underlying source but it only sets the named export if the CJS exports hasOwnProperty. This trap will allow the proxy to respond affirmatively.

I did not add unit tests because contriving the ESM <-> CJS scenario in Jest is challenging. I did add new components to the flight fixture which demonstrate that the named exports are properly constructed with the client reference whereas they were not before.

… practical place where this shows up is when consuming CJS module.exports in ESM modules. Node creates named exports it statically infers from the underlying source but it only sets the named export if the CJS exports hasOwnProperty. This trap will allow the proxy to respond affirmatively.
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Sep 2, 2023
@react-sizebot
Copy link

Comparing: d23b8b5...a833972

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.min.js = 165.63 kB 165.63 kB = 51.88 kB 51.88 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 174.70 kB 174.70 kB = 54.61 kB 54.61 kB
facebook-www/ReactDOM-prod.classic.js = 570.44 kB 570.44 kB = 100.45 kB 100.45 kB
facebook-www/ReactDOM-prod.modern.js = 554.21 kB 554.21 kB = 97.61 kB 97.61 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.browser.production.min.js +0.88% 25.32 kB 25.54 kB +0.68% 8.62 kB 8.68 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.browser.production.min.js +0.88% 25.32 kB 25.54 kB +0.68% 8.62 kB 8.68 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.edge.production.min.js +0.87% 25.64 kB 25.87 kB +0.72% 8.72 kB 8.79 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.edge.production.min.js +0.87% 25.64 kB 25.87 kB +0.72% 8.72 kB 8.79 kB
oss-stable-semver/react-server-dom-webpack/umd/react-server-dom-webpack-server.browser.production.min.js +0.85% 25.50 kB 25.72 kB +0.95% 8.72 kB 8.81 kB
oss-stable/react-server-dom-webpack/umd/react-server-dom-webpack-server.browser.production.min.js +0.85% 25.50 kB 25.72 kB +0.95% 8.72 kB 8.81 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.browser.production.min.js +0.84% 26.53 kB 26.76 kB +0.69% 8.92 kB 8.99 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.production.min.js +0.84% 26.58 kB 26.81 kB +0.73% 9.06 kB 9.13 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.production.min.js +0.84% 26.58 kB 26.81 kB +0.73% 9.06 kB 9.13 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.edge.production.min.js +0.83% 26.85 kB 27.08 kB +0.73% 9.02 kB 9.09 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.production.min.js +0.83% 27.11 kB 27.33 kB +0.68% 9.22 kB 9.28 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.production.min.js +0.83% 27.11 kB 27.33 kB +0.68% 9.22 kB 9.28 kB
oss-experimental/react-server-dom-webpack/umd/react-server-dom-webpack-server.browser.production.min.js +0.81% 26.72 kB 26.93 kB +0.96% 9.03 kB 9.11 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.production.min.js +0.81% 27.74 kB 27.96 kB +0.69% 9.34 kB 9.41 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.production.min.js +0.79% 28.27 kB 28.49 kB +0.63% 9.51 kB 9.57 kB
oss-stable-semver/react-server-dom-webpack/umd/react-server-dom-webpack-server.browser.development.js +0.35% 103.79 kB 104.15 kB +0.55% 24.00 kB 24.13 kB
oss-stable/react-server-dom-webpack/umd/react-server-dom-webpack-server.browser.development.js +0.35% 103.79 kB 104.15 kB +0.55% 24.00 kB 24.13 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.browser.development.js +0.34% 98.15 kB 98.49 kB +0.52% 23.63 kB 23.75 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.browser.development.js +0.34% 98.15 kB 98.49 kB +0.52% 23.63 kB 23.75 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.edge.development.js +0.34% 98.56 kB 98.90 kB +0.51% 23.74 kB 23.86 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.edge.development.js +0.34% 98.56 kB 98.90 kB +0.51% 23.74 kB 23.86 kB
oss-experimental/react-server-dom-webpack/umd/react-server-dom-webpack-server.browser.development.js +0.34% 108.89 kB 109.25 kB +0.52% 24.88 kB 25.01 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.development.js +0.33% 101.86 kB 102.20 kB +0.50% 24.21 kB 24.33 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.development.js +0.33% 101.86 kB 102.20 kB +0.50% 24.21 kB 24.33 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.browser.development.js +0.33% 103.01 kB 103.35 kB +0.49% 24.52 kB 24.64 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.edge.development.js +0.32% 103.42 kB 103.76 kB +0.49% 24.65 kB 24.77 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.development.js +0.32% 103.90 kB 104.24 kB +0.49% 24.82 kB 24.95 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.development.js +0.32% 103.90 kB 104.24 kB +0.49% 24.82 kB 24.95 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.development.js +0.32% 106.27 kB 106.61 kB +0.51% 24.97 kB 25.10 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.development.js +0.31% 108.32 kB 108.66 kB +0.50% 25.56 kB 25.69 kB

Generated by 🚫 dangerJS against a833972

@gnoff gnoff merged commit b9be453 into facebook:main Sep 5, 2023
2 checks passed
@gnoff gnoff deleted the proxy-descriptors branch September 5, 2023 20:45
@anupam-singh16
Copy link

hi my name is anupam singh...i want contribution code in react ..so please suggest me how to add our code deploy on production

sebmarkbage added a commit that referenced this pull request Feb 16, 2024
…28352)

The newer version triggers an error due to require() not being compiled.

This was upgraded in #27328. I'm not sure why that was needed. Maybe it
was just because it was allowed by the caret and something else upgraded
but I haven't been able to make it work with the newer version. So I'll
just pin it.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…#27328)

Client reference proxy should implement getOwnPropertyDescriptor. One
practical place where this shows up is when consuming CJS module.exports
in ESM modules. Node creates named exports it statically infers from the
underlying source but it only sets the named export if the CJS exports
hasOwnProperty. This trap will allow the proxy to respond affirmatively.

I did not add unit tests because contriving the ESM <-> CJS scenario in
Jest is challenging. I did add new components to the flight fixture
which demonstrate that the named exports are properly constructed with
the client reference whereas they were not before.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…acebook#28352)

The newer version triggers an error due to require() not being compiled.

This was upgraded in facebook#27328. I'm not sure why that was needed. Maybe it
was just because it was allowed by the caret and something else upgraded
but I haven't been able to make it work with the newer version. So I'll
just pin it.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
Client reference proxy should implement getOwnPropertyDescriptor. One
practical place where this shows up is when consuming CJS module.exports
in ESM modules. Node creates named exports it statically infers from the
underlying source but it only sets the named export if the CJS exports
hasOwnProperty. This trap will allow the proxy to respond affirmatively.

I did not add unit tests because contriving the ESM <-> CJS scenario in
Jest is challenging. I did add new components to the flight fixture
which demonstrate that the named exports are properly constructed with
the client reference whereas they were not before.

DiffTrain build for commit b9be453.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
…28352)

The newer version triggers an error due to require() not being compiled.

This was upgraded in #27328. I'm not sure why that was needed. Maybe it
was just because it was allowed by the caret and something else upgraded
but I haven't been able to make it work with the newer version. So I'll
just pin it.

DiffTrain build for commit 2ba1b78.
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.

5 participants