Remove deprecated wildcard folder mapping #23256
Conversation
acdlite
changed the title
packages/react-dom/package.json
Outdated
| "./package.json": "./package.json", | ||
| "./": "./" | ||
| "./unstable_testing": "./unstable_testing.js", | ||
| "./umd/*": "./umd/*", |
There was a problem hiding this comment.
I kept the UMD bundles reachable for now until we make a final decision on when to remove them
There was a problem hiding this comment.
So, my thinking about the umd bundles is that even if you do use them, you typically don't use them through a require implementation that goes through exports. Like you could technically require them but you're not supposed to. I think we can remove these and then if we get a report that's legit maybe add them back in a patch/minor?
acdlite
force-pushed
the
remove-deprecated-wildcard
branch
3 times, most recently
from
e667998
to
2fb3805
packages/react-dom/package.json
Outdated
| @@ -50,8 +50,10 @@ | |||
| "./profiling": "./profiling.js", | |||
| "./test-utils": "./test-utils.js", | |||
| "./unstable_testing": "./unstable_testing.js", | |||
| "./package.json": "./package.json", | |||
| "./": "./" | |||
| "./testing": "./testing.js", | |||
There was a problem hiding this comment.
This is an experimental package. It got accidentally added to the @next release channel. We don't intend to release to 18 stable. I'll remove it in a follow-up.
There was a problem hiding this comment.
Will be fixed by @sebmarkbage in #23258
There was a problem hiding this comment.
If you're rebased on mine, can you remove this now?
|
Comparing: 274b9fb...c281dd9 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) |
acdlite
force-pushed
the
remove-deprecated-wildcard
branch
3 times, most recently
from
4db18bc
to
ee66a87
| @@ -27,7 +27,9 @@ | |||
| "./package.json": "./package.json", | |||
| "./jsx-runtime": "./jsx-runtime.js", | |||
| "./jsx-dev-runtime": "./jsx-dev-runtime.js", | |||
| "./": "./" | |||
| "./umd/*": "./umd/*", | |||
| "./unstable-shared-subset": "./unstable-shared-subset.js" | |||
There was a problem hiding this comment.
Instead of exposing this, can we instead just use the deep linking into /src/ for the bundle? Because that way it's not exposed upon publishing.
Also, deleting all ./src/* entries after bundling is easier than any arbitrary name.
There was a problem hiding this comment.
Yeah that sounds good
There was a problem hiding this comment.
I added the name field to bundles for this purpose since otherwise they'd get the deep link name. If I had that at the time I don't think I would've added this top level entry point.
There was a problem hiding this comment.
I'm going to open a separate PR for this so I can get it reviewed
What's you plan for the ./src/* entry in published package.json? Just leave it in even though the files are not there or remove post-build?
Planning to remove it post-build since the Rollup script still needs it |
Node v16 deprecated the use of trailing "/" to define subpath folder mappings in the "exports" field of package.json. The recommendation is to explicitly list all our exports. We already do that for all our public modules. I believe the only reason we have a wildcard pattern is because our package.json files are also used at build time (by Rollup) to resolve internal source modules that don't appear in the final npm artifact. Changing trailing "/" to "/*" fixes the warnings. See https://nodejs.org/api/packages.html#subpath-patterns for more info. Since the wildcard pattern only exists so our build script has access to internal at build time, I've scoped the wildcard to "/src/*". Because our public modules are located outside the "src" directory, this means deep imports of our modules will no longer work: only packages that are listed in the "exports" field. The only two affected packages are react-dom and react. We need to be sure that all our public modules are still reachable. I audited the exports by comparing the entries to the "files" field in package.json, which represents a complete list of the files that are included in the final release artifact. At some point, we should add an e2e packaging test to prevent regressions; for now, we should have decent coverage because in CI we run our Jest test suite against the release artifacts.
Our expectation is that if you're using the UMD builds, you're not loading them through a normal module system like require or import. Instead you're probably copying the files directly or loading them from a CDN like unpkg.
Node v16 deprecated the use of trailing "/" to define subpath folder mappings in the "exports" field of package.json.
The recommendation is to explicitly list all our exports. We already do that for all our public modules. I believe the only reason we have a wildcard pattern is because our package.json files are also used at build time (by Rollup) to resolve internal source modules that don't appear in the final npm artifact.
Changing trailing "/" to "/*" fixes the warnings. See https://nodejs.org/api/packages.html#subpath-patterns for more info.
Since the wildcard pattern only exists so our build script has access to internal at build time, I've scoped the wildcard to "/src/*". Because our public modules are located outside the "src" directory, this means deep imports of our modules will no longer work: only packages that are listed in the "exports" field.
The only two affected packages are react-dom and react. We need to be sure that all our public modules are still reachable. I audited the exports by comparing the entries to the "files" field in package.json, which represents a complete list of the files that are included in the final release artifact.
At some point, we should add an e2e packaging test to prevent regressions; for now, we should have decent coverage because in CI we run our Jest test suite against the release artifacts.
The text was updated successfully, but these errors were encountered: