Export React as Named Exports instead of CommonJS #18106
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
React Core Team
|
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 cb4ff7b:
|
Details of bundled changes.Comparing: 8d7535e...cb4ff7b react-dom
react
ReactDOM: size: 0.0%, gzip: -0.3% React: size: 🔺+0.3%, gzip: -0.1% Size changes (experimental) |
Details of bundled changes.Comparing: 8d7535e...cb4ff7b react-dom
react
ReactDOM: size: 0.0%, gzip: -0.1% React: size: 🔺+0.3%, gzip: 0.0% Size changes (stable) |
|
The increase in pre-gzip size for the react package is because this causes Rollup to encode this as: Instead of: Which I guess is more correct in a sense but unclear what is actually better for perf. This probably doesn't matter much since gzip takes care of it and the other packages won't matter as much since they have much fewer exports. |
sebmarkbage
force-pushed
the
namedexports
branch
2 times, most recently
from
bf4a7b9 to
4d8d954
Compare
acdlite
approved these changes
Feb 24, 2020
|
Apologies for jumping in here without full context, but I'm a bit worried about switching to only named exports for react and react-dom, which appears to be the direction the ESM bundling is going. Isn't that a hugely breaking change? The following code is so very common: import ReactDOM from 'react-dom';
import React from 'react';
React.createElement('div')
ReactDOM.render(...)For those wishing to use the ESM bundle once it's finished, will they have to migrate all code to use named exports? Is a codemod planned to help with migration? Or am I misunderstanding this entirely? |
We currently fork .fb.js entry points. This adds a few more options. .modern.fb.js - experimental FB builds .classic.fb.js - stable FB builds .fb.js - if no other FB build, use this for FB builds .experimental.js - experimental builds .stable.js - stable builds .js - used if no other override exists This will be used to have different ES exports for different builds.
We need to re-export the Flow exported types so we can use them in our code. We don't want to use the Flow types from upstream since it doesn't have the non-public APIs that we have. This should be able to use export * but I don't know why it doesn't work. This actually enables Flow typing of React which was just "any" before. This exposed some Flow errors that needs fixing.
None of our builds expose all exports and they all differ in at least one way, so we need four forks.
We don't want to emit the esModule compatibility flag on our CommonJS output. For now we treat our named exports as if they're CommonJS. This is a potentially breaking change for scheduler (but all those apis are unstable), react-is and use-subscription. However, it seems unlikely that anyone would rely on this since these only have named exports.
This lets it do the negative test by ensuring that the right tests fail. However, this in turn will make other tests that are not behind __EXPERIMENTAL__ fail. So I need to do that next.
Since there's no way to override the exports using feature flags in .intern.js anymore we can't use these APIs in stable. The tradeoff here is that we can either enable the negative tests on "stable" that means experimental are expected to fail, or we can disable tests on stable. This is unfortunate since some of these APIs now run on a "stable" config at FB instead of the experimental.
Same strategy as React. I moved the ReactDOMFB runtime injection to classic.fb.js Since we only fork the entrypoint, the `/testing` entrypoint needs to be forked too to re-export the same things plus `act`. This is a bit unfortunate. If it becomes a pattern we can consider forking in the module resolution deeply. fix flow
Now that ReactDOM is Flow type checked we need to fix up its types.
These are only flagging the exports and no implementation details so we can control them fully through the export overrides.
sebmarkbage
force-pushed
the
namedexports
branch
from
4d8d954 to
cb4ff7b
Compare
|
@joeldenning Don't worry. This is commonjs only change. ESM will have separate entry points with with default exports. |
|
@TrySound, do you know when the change is planned for? Specifically, when will react comes with an ESM variant alongside CJS and UMD ones? |
|
No idea. Probably when conditional exports will be widely supported. Either in react 18 or 19. |
|
Thanks for your answer. I'm wondering if it would be possible to just provide the sources as-is in the published packages, alongside the "compiled" CJS and UMD variants. This way we could simple import from the sources and create our own application using ES syntax. |
|
Sources will not help. There are a lot of configurations on top of it like flow syntax, aliases to shared package which is not published and many more. |
|
It's sad. Sounds like we are years away to get rid of bundling to build a react app. I am perfectly aware that bundling is here to stay because we have a long way to go before ES modules are widely adopted by all browsers, but I'm talking here about not bundling when developing - it's not needed and bundling impairs the performance a lot for developers. Is it planned that React migrate to TypeScript at one point? |
|
Snowpack allows to not use bundler for development. And their service has ready to use esm version of react bundle. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal is to ensure more consistency in our tooling (e.g. we didn't even Flow check any imports of React in our code), better compiler output and unblock the ability to publish ES modules.
We can't conditionally export names, so we can't use feature flags to determine the exports anymore.
Instead, in my approach the default index.js exports all exports. This allows Flow to check and tests based on flags to get all features. Then I use a file extension scheme to fork the exports based on the four build types we need.
I'd review the individual commit messages.
The output builds:
Stable - sebmarkbage/react-builds@ed86c04
Experimental - sebmarkbage/react-builds@5c0f29c