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

Support named exports from client references #20312

Merged
merged 8 commits into from Nov 30, 2020

Conversation

@sebmarkbage
Copy link
Member

@sebmarkbage sebmarkbage commented Nov 21, 2020

Module references now include the "export" name that they're importing. Each export is modeled as its own reference. It's possible with module splitting that different exports end up in different chunks. It's also possible that the export is renamed as part of minification. So the map also includes a map from the original to the bundled name.

In the CommonJS runtime we can't statically analyze the exports so instead we use Proxy and depending on what the server reads, we emit a different reference.

If the ESM runtime we can statically analyze the exports but it's a little tricky because export * is recursive.

There are weird combinations that can happen because Babel may or may not convert things to CJS from ESM on either the server or the client bundler.

if (moduleData.name === '*') {
// This is a placeholder value that represents that the caller imported this
// as a CommonJS module as is.
return moduleExports;

This comment has been minimized.

@sebmarkbage

sebmarkbage Nov 21, 2020
Author Member

I usually test CJS server paths by removing the "type": "module" definition in package.json. In this case I need to use raw CJS (require(...) + module.exports) without using Babel/Webpack. Because otherwise we fall into the case below.

I couldn't test the Webpack client paths for true CJS. I spent a few hours trying to figure out how to make the react-script config let me write raw CJS instead of transpiling from ESM and I couldn't because webpack kept treating them as ESM which means that assigning to module.exports causes and error.

I think this works though.

@codesandbox
Copy link

@codesandbox codesandbox bot commented Nov 21, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a5f7228:

Sandbox Source
React Configuration
@sizebot
Copy link

@sizebot sizebot commented Nov 21, 2020

Details of bundled changes.

Comparing: 565148d...a5f7228

react-transport-dom-webpack

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-transport-dom-webpack-server.browser.development.js +0.2% +0.3% 24.07 KB 24.12 KB 6.59 KB 6.61 KB NODE_DEV
react-transport-dom-webpack-node-register.js +118.4% +75.7% 1.48 KB 3.24 KB 794 B 1.36 KB NODE_ES2015
react-transport-dom-webpack-server.browser.production.min.js 🔺+0.4% 🔺+0.6% 6.07 KB 6.1 KB 2.53 KB 2.55 KB NODE_PROD
react-transport-dom-webpack.development.js +2.8% +2.5% 17.54 KB 18.03 KB 4.83 KB 4.96 KB UMD_DEV
react-transport-dom-webpack.production.min.js 🔺+1.4% 🔺+1.3% 3.9 KB 3.95 KB 1.75 KB 1.78 KB UMD_PROD
react-transport-dom-webpack.development.js +2.8% +2.5% 16.35 KB 16.81 KB 4.72 KB 4.84 KB NODE_DEV
react-transport-dom-webpack.production.min.js 🔺+1.5% 🔺+1.5% 3.7 KB 3.75 KB 1.66 KB 1.68 KB NODE_PROD
react-transport-dom-webpack-server.browser.development.js +0.2% +0.2% 25.64 KB 25.69 KB 6.72 KB 6.74 KB UMD_DEV
react-transport-dom-webpack-server.node.development.js +0.2% +0.3% 25.01 KB 25.06 KB 6.85 KB 6.86 KB NODE_DEV
react-transport-dom-webpack-server.browser.production.min.js 🔺+0.4% 🔺+0.8% 6.28 KB 6.31 KB 2.62 KB 2.64 KB UMD_PROD
react-transport-dom-webpack-server.node.production.min.js 🔺+0.4% 🔺+0.5% 6.27 KB 6.29 KB 2.56 KB 2.57 KB NODE_PROD
react-transport-dom-webpack-node-loader.js +150.7% +82.3% 3.29 KB 8.26 KB 1.54 KB 2.81 KB NODE_ESM
react-transport-dom-webpack-plugin.js 0.0% -0.3% 437 B 437 B 299 B 298 B NODE_ES2015

Size changes (experimental)

Generated by 🚫 dangerJS against a5f7228

yarn.lock Outdated
version "6.4.2"
resolved "https://registry.yarnpkg.com/acorn/-/acorn-6.4.2.tgz#35866fd710528e92de10cf06016498e47e39e1e6"
integrity sha512-XtGIhXwF8YM8bJhGxG5kXgjkEuNGLTkoYqVE+KMR+aspr4KGYmKYg7yUe3KghyQ9yheNwLnjmzh/7+gfDBmHCQ==

acorn@^6.4.1:

This comment has been minimized.

@sebmarkbage

sebmarkbage Nov 21, 2020
Author Member

Not sure why yarn resolves to so many different acorns but meh.

This comment has been minimized.

@sizebot
Copy link

@sizebot sizebot commented Nov 21, 2020

Details of bundled changes.

Comparing: 565148d...a5f7228

react-transport-dom-webpack

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-transport-dom-webpack-server.browser.development.js +0.2% +0.3% 24.05 KB 24.11 KB 6.58 KB 6.6 KB NODE_DEV
react-transport-dom-webpack-node-register.js +119.5% +76.5% 1.47 KB 3.23 KB 784 B 1.35 KB NODE_ES2015
react-transport-dom-webpack-server.browser.production.min.js 🔺+0.4% 🔺+0.6% 6.06 KB 6.09 KB 2.52 KB 2.54 KB NODE_PROD
react-transport-dom-webpack.development.js +2.8% +2.5% 17.53 KB 18.02 KB 4.82 KB 4.95 KB UMD_DEV
react-transport-dom-webpack.production.min.js 🔺+1.4% 🔺+1.3% 3.89 KB 3.94 KB 1.75 KB 1.77 KB UMD_PROD
react-transport-dom-webpack.development.js +2.9% +2.5% 16.34 KB 16.8 KB 4.71 KB 4.83 KB NODE_DEV
react-transport-dom-webpack.production.min.js 🔺+1.5% 🔺+1.5% 3.68 KB 3.74 KB 1.65 KB 1.67 KB NODE_PROD
react-transport-dom-webpack-server.browser.development.js +0.2% +0.2% 25.63 KB 25.68 KB 6.71 KB 6.73 KB UMD_DEV
react-transport-dom-webpack-server.node.development.js +0.2% +0.3% 25 KB 25.05 KB 6.84 KB 6.85 KB NODE_DEV
react-transport-dom-webpack-server.browser.production.min.js 🔺+0.4% 🔺+0.8% 6.27 KB 6.3 KB 2.61 KB 2.63 KB UMD_PROD
react-transport-dom-webpack-server.node.production.min.js 🔺+0.4% 🔺+0.5% 6.25 KB 6.28 KB 2.55 KB 2.56 KB NODE_PROD
react-transport-dom-webpack-node-loader.js +151.3% +82.9% 3.28 KB 8.24 KB 1.53 KB 2.8 KB NODE_ESM
react-transport-dom-webpack-plugin.js 0.0% -0.3% 424 B 424 B 290 B 289 B NODE_ES2015

Size changes (stable)

Generated by 🚫 dangerJS against a5f7228

Copy link
Member

@gaearon gaearon left a comment

This is getting complicated. Seems like getting a good integration test fixture would be a necessary part of productionizing.

@sebmarkbage
Copy link
Member Author

@sebmarkbage sebmarkbage commented Nov 23, 2020

I’m going to punt on a lot of tests because this is just the beginning. Someone else can help with getting this to the finish line. I need to focus on getting the shell in place.

sebmarkbage and others added 8 commits Nov 21, 2020
This field name will get confused with the imported name or the module id.
getSource would be more efficient in the cases where we don't need to read
the original file but we'll need to most of the time.

Even then, we can't return a JS file if we're trying to support non-JS
loader because it'll end up being transformed.

Similarly, we'll need to parse the file and we can't parse it before it's
transformed. So we need to chain with other loaders that know how.
This should be the version used by Webpack since we have a dependency on
Webpack anyway.
We need to statically resolve the names that a client component will
export so that we can export a module reference for each of the names.

For export * from, this gets tricky because we need to also load the
source of the next file to parse that. We don't know exactly how the
client is built so we guess it's somewhat default.
We use a proxy to see what property the server access and that will tell
us which property we'll want to import on the client.
To support named exports each name needs to be encoded as a separate
reference. It's possible with module splitting that different exports end
up in different chunks.

It's also possible that the export is renamed as part of minification.
So the map also includes a map from the original to the bundled name.
This models if the server tries to import .default or a plain require.
We should replicate the same thing on the client when we load that
module reference.
@sebmarkbage sebmarkbage force-pushed the sebmarkbage:clientnamedexports branch from f11fc66 to a5f7228 Nov 30, 2020
@sebmarkbage sebmarkbage merged commit 3f73dce into facebook:master Nov 30, 2020
34 checks passed
34 checks passed
Facebook CLA Check Contributor License Agreement is valid!
Details
ci/circleci: RELEASE_CHANNEL_stable_yarn_build Your tests passed on CircleCI!
Details
ci/circleci: RELEASE_CHANNEL_stable_yarn_lint_build Your tests passed on CircleCI!
Details
ci/circleci: RELEASE_CHANNEL_stable_yarn_test_dom_fixtures Your tests passed on CircleCI!
Details
ci/circleci: build_devtools_and_process_artifacts Your tests passed on CircleCI!
Details
ci/circleci: build_devtools_scheduling_profiler Your tests passed on CircleCI!
Details
ci/circleci: process_artifacts Your tests passed on CircleCI!
Details
ci/circleci: process_artifacts_experimental Your tests passed on CircleCI!
Details
ci/circleci: setup Your tests passed on CircleCI!
Details
ci/circleci: sizebot_experimental Your tests passed on CircleCI!
Details
ci/circleci: sizebot_stable Your tests passed on CircleCI!
Details
ci/circleci: yarn_build Your tests passed on CircleCI!
Details
ci/circleci: yarn_flow Your tests passed on CircleCI!
Details
ci/circleci: yarn_lint Your tests passed on CircleCI!
Details
ci/circleci: yarn_lint_build Your tests passed on CircleCI!
Details
ci/circleci: yarn_test Your tests passed on CircleCI!
Details
ci/circleci: yarn_test-classic Your tests passed on CircleCI!
Details
ci/circleci: yarn_test-classic_prod Your tests passed on CircleCI!
Details
ci/circleci: yarn_test-classic_prod_variant Your tests passed on CircleCI!
Details
ci/circleci: yarn_test-classic_variant Your tests passed on CircleCI!
Details
ci/circleci: yarn_test-stable Your tests passed on CircleCI!
Details
ci/circleci: yarn_test-stable_build Your tests passed on CircleCI!
Details
ci/circleci: yarn_test-stable_build_prod Your tests passed on CircleCI!
Details
ci/circleci: yarn_test-stable_persistent Your tests passed on CircleCI!
Details
ci/circleci: yarn_test-stable_prod Your tests passed on CircleCI!
Details
ci/circleci: yarn_test-www Your tests passed on CircleCI!
Details
ci/circleci: yarn_test-www_prod Your tests passed on CircleCI!
Details
ci/circleci: yarn_test-www_prod_variant Your tests passed on CircleCI!
Details
ci/circleci: yarn_test-www_variant Your tests passed on CircleCI!
Details
ci/circleci: yarn_test_build Your tests passed on CircleCI!
Details
ci/circleci: yarn_test_build_devtools Your tests passed on CircleCI!
Details
ci/circleci: yarn_test_build_prod Your tests passed on CircleCI!
Details
ci/circleci: yarn_test_prod Your tests passed on CircleCI!
Details
ci/codesandbox Building packages succeeded.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.