Add exports field to react-dom (including configuring Cloudflare and Deno) #23252
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
acdlite
approved these changes
Feb 8, 2022
There was a problem hiding this comment.
This seems like the aspirational goal we want though and to avoid breaking changes it seems worth landing. For example, Deno in Node compat mode should still use the browser build.
I think we should do a full e2e test: set-up fake npm registry, install packages, run a smoke test. Don't need to run it on every commit, maybe just before a release or in a cron job.
sebmarkbage
commented
Feb 8, 2022
| "./profiling": "./profiling.js", | ||
| "./test-utils": "./test-utils.js", | ||
| "./package.json": "./package.json", | ||
| "./": "./" |
There was a problem hiding this comment.
We know that this pattern is deprecated and it'll be removed from builds in another PR.
|
Comparing: 9d4e8e8...f8986bb Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
This configures the exports field for react-dom. Notably there are some conditions for the /server entry point where we pick the export based on environments. Most environments now support Web Streams which is preferred in those environments. We already do this using the "browser" field so the "browser" condition applies here too. I don't think it's necessary, but I also specified "worker" explicitly since this is for Service Workers and those are often targeted with Web Pack's "webworker" target, which is also what Cloudflare currently recommends. I also added "deno" but deno is a bit special because this only works if you run with the node compatibility since otherwise you have to specify absolute URLs for the imports.
sebmarkbage
force-pushed
the
reactdomexports
branch
from
82126b8
to
f8986bb
Compare
|
The issue with Deno is that the idiomatic way to consume react is probably through intermediate hosts that rewrite them to ESM automatically. Such as this one: https://jspm.dev/react@next So a fake npm registry wouldn't even be enough. |
zhengjitf
pushed a commit
to zhengjitf/react
that referenced
this pull request
Apr 15, 2022
This configures the exports field for react-dom. Notably there are some conditions for the /server entry point where we pick the export based on environments. Most environments now support Web Streams which is preferred in those environments. We already do this using the "browser" field so the "browser" condition applies here too. I don't think it's necessary, but I also specified "worker" explicitly since this is for Service Workers and those are often targeted with Web Pack's "webworker" target, which is also what Cloudflare currently recommends. I also added "deno" but deno is a bit special because this only works if you run with the node compatibility since otherwise you have to specify absolute URLs for the imports.
Closed
1 task
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.
This configures the exports field for react-dom.
Notably there are some conditions for the /server entry point where we pick the export based on environments. Most environments now support Web Streams which is preferred in those environments.
We already do this using the "browser" field so the "browser" condition applies here too.
I also specified "worker" explicitly since this is for Service Workers and those are often targeted with Webpack's "webworker" target, which is also what Cloudflare currently recommends. I don't think this is necessary because Webpack includes the "browser" field for this case. However, it seems more accurate. Although I'm not sure if this would break if you use a Worker in Node.js.
I also added "deno" but Deno is a bit special because this only works if you run with the node compatibility on since otherwise you have to specify absolute URLs for the imports anyway. Currently the node compatibility doesn't work well from real ESM and the built-in JSX support doesn't work outside of real ESM. So this is a bit of a mess.
I wasn't able to add fixtures and test any of this yet. It ended up being kind of messy and a lot of work to create a real environment. This seems like the aspirational goal we want though and to avoid breaking changes it seems worth landing. For example, Deno in Node compat mode should still use the browser build.