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

Use undici polyfill for tests in old Node versions #28887

Merged
merged 1 commit into from
May 3, 2024

Conversation

sebmarkbage
Copy link
Collaborator

We currently don't test FormData / File dependent features in CI because we use an old Node.js version in CI. We should probably upgrade to 18 since that's really the minimum version that supports all the features out of the box.

JSDOM is not a faithful/compatible implementation of these APIs. The recommended way to use Flight together with FormData/Blob/File in older Node.js versions, is to polyfill using the undici library.

However, even in these versions the Blob implementation isn't quite faithful so the Reply client needs a slight tweak for multi-byte typed arrays.

@sebmarkbage sebmarkbage requested a review from gnoff April 21, 2024 18:12
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Apr 21, 2024
@eps1lon
Copy link
Collaborator

eps1lon commented Apr 21, 2024

We should probably upgrade to 18 since that's really the minimum version that supports all the features out of the box.

CI should be running with 18.20 as of 50895bc

@react-sizebot
Copy link

react-sizebot commented Apr 22, 2024

Comparing: 0a0a3af...94add5a

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-stable/react-dom/cjs/react-dom-client.production.js = 492.61 kB 492.61 kB = 87.88 kB 87.88 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB = 1.83 kB 1.82 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 498.86 kB 498.86 kB = 88.92 kB 88.92 kB
facebook-www/ReactDOM-prod.classic.js = 591.22 kB 591.22 kB = 103.96 kB 103.96 kB
facebook-www/ReactDOM-prod.modern.js = 567.44 kB 567.44 kB = 100.36 kB 100.36 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-server-dom-esm/cjs/react-server-dom-esm-client.browser.production.js +0.87% 40.07 kB 40.42 kB +0.50% 8.42 kB 8.46 kB
oss-experimental/react-client/cjs/react-client-flight.production.js +0.85% 42.61 kB 42.97 kB +0.51% 8.37 kB 8.41 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.browser.production.js +0.84% 41.62 kB 41.96 kB +0.54% 8.78 kB 8.83 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.production.js +0.83% 41.95 kB 42.30 kB +0.47% 8.88 kB 8.92 kB
oss-experimental/react-server-dom-esm/cjs/react-server-dom-esm-client.node.production.js +0.78% 44.46 kB 44.81 kB +0.44% 9.47 kB 9.51 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.unbundled.production.js +0.76% 45.71 kB 46.06 kB +0.46% 9.74 kB 9.79 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.unbundled.production.js +0.76% 45.71 kB 46.06 kB +0.45% 9.75 kB 9.79 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.production.js +0.75% 46.39 kB 46.74 kB +0.42% 9.92 kB 9.96 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.production.js +0.75% 46.40 kB 46.75 kB +0.41% 9.90 kB 9.95 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.production.js +0.73% 47.43 kB 47.77 kB +0.44% 10.11 kB 10.15 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.edge.production.js +0.73% 47.44 kB 47.78 kB +0.43% 10.10 kB 10.14 kB
oss-experimental/react-server-dom-esm/esm/react-server-dom-esm-client.browser.production.js +0.71% 64.50 kB 64.96 kB +0.92% 13.97 kB 14.09 kB
oss-experimental/react-client/cjs/react-client-flight.development.js +0.56% 81.32 kB 81.77 kB +0.75% 18.45 kB 18.59 kB
oss-experimental/react-server-dom-esm/esm/react-server-dom-esm-client.browser.development.js +0.56% 81.32 kB 81.78 kB +0.78% 18.16 kB 18.30 kB
oss-experimental/react-server-dom-esm/cjs/react-server-dom-esm-client.browser.development.js +0.56% 81.56 kB 82.02 kB +0.77% 18.23 kB 18.37 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.browser.development.js +0.54% 84.81 kB 85.27 kB +0.72% 19.20 kB 19.34 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js +0.54% 85.32 kB 85.78 kB +0.70% 19.38 kB 19.51 kB
oss-experimental/react-server-dom-esm/cjs/react-server-dom-esm-client.node.development.js +0.51% 89.24 kB 89.70 kB +0.73% 20.26 kB 20.40 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.unbundled.development.js +0.50% 91.02 kB 91.48 kB +0.68% 20.83 kB 20.97 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.unbundled.development.js +0.50% 91.06 kB 91.52 kB +0.69% 20.85 kB 21.00 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.development.js +0.50% 92.46 kB 92.92 kB +0.65% 21.21 kB 21.35 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.development.js +0.50% 92.49 kB 92.94 kB +0.65% 21.25 kB 21.39 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.edge.development.js +0.49% 93.53 kB 93.99 kB +0.64% 21.41 kB 21.55 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js +0.49% 93.56 kB 94.01 kB +0.65% 21.45 kB 21.59 kB
test_utils/ReactAllWarnings.js Deleted 64.26 kB 0.00 kB Deleted 16.02 kB 0.00 kB

Generated by 🚫 dangerJS against 94add5a

@sebmarkbage
Copy link
Collaborator Author

This is still valuable to test in old Node versions manually. I guess the question is, which versions do we support?

const blob = new Blob([typedArray]);
const blob = new Blob([
// We should be able to pass the buffer straight through but Node < 18 treat
// multi-byte array blobs differently so we first convert it to single-byte.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are only relevant in Server-to-Server calls and maybe SSR serialization of forms. The question is do we support old Node without polyfilling?

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO we should only support supported versions of Node so we can probably assume FormData

To support old Node we need to adjust how we construct blobs.
@sebmarkbage sebmarkbage merged commit 5fcfd71 into facebook:main May 3, 2024
38 checks passed
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