Remove deprecated wildcard folder mapping #23256

acdlite · 2022-02-09T00:58:44Z

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.

acdlite · 2022-02-09T01:00:25Z

packages/react-dom/package.json

-    "./package.json": "./package.json",
-    "./": "./"
+    "./unstable_testing": "./unstable_testing.js",
+    "./umd/*": "./umd/*",


I kept the UMD bundles reachable for now until we make a final decision on when to remove them

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 · 2022-02-09T02:48:23Z

packages/react-dom/package.json

@@ -50,8 +50,10 @@
    "./profiling": "./profiling.js",
    "./test-utils": "./test-utils.js",
    "./unstable_testing": "./unstable_testing.js",
-    "./package.json": "./package.json",
-    "./": "./"
+    "./testing": "./testing.js",


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.

Will be fixed by @sebmarkbage in #23258

If you're rebased on mine, can you remove this now?

sizebot · 2022-02-09T02:50:01Z

Comparing: 274b9fb...c281dd9

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.min.js	=	130.35 kB	130.35 kB	=	41.82 kB	41.82 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	135.53 kB	135.53 kB	=	43.35 kB	43.35 kB
facebook-www/ReactDOM-prod.classic.js	=	431.14 kB	431.14 kB	=	79.11 kB	79.11 kB
facebook-www/ReactDOM-prod.modern.js	=	421.08 kB	421.08 kB	=	77.68 kB	77.68 kB
facebook-www/ReactDOMForked-prod.classic.js	=	431.14 kB	431.14 kB	=	79.11 kB	79.11 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against c281dd9

sebmarkbage · 2022-02-09T16:44:31Z

packages/react/package.json

@@ -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"


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.

Yeah that sounds good

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.

I'm going to open a separate PR for this so I can get it reviewed

sebmarkbage

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?

acdlite · 2022-02-09T16:47:10Z

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.

Summary: This sync includes the following changes: - **[27b569969](facebook/react@27b569969 )**: Simplify cache pool contexts ([#23280](facebook/react#23280)) //<Andrew Clark>// - **[1fb0d0687](facebook/react@1fb0d0687 )**: [Devtools][Transition Tracing] Add Transition callbacks to createRoot ([#23276](facebook/react#23276)) //<Luna Ruan>// - **[a6987bee7](facebook/react@a6987bee7 )**: add <TracingMarker> component boilerplate ([#23275](facebook/react#23275)) //<Luna Ruan>// - **[796fff548](facebook/react@796fff548 )**: Allow suspending outside a Suspense boundary ([#23267](facebook/react#23267)) //<Andrew Clark>// - **[64223fed8](facebook/react@64223fed8 )**: Fix: Multiple hydration errors in same render ([#23273](facebook/react#23273)) //<Andrew Clark>// - **[efd8f6442](facebook/react@efd8f6442 )**: Resolve default onRecoverableError at root init ([#23264](facebook/react#23264)) //<Andrew Clark>// - **[e0af1aabe](facebook/react@e0af1aabe )**: Fix wrong context argument to `apply` //<Andrew Clark>// - **[9b5e0517b](facebook/react@9b5e0517b )**: Remove deprecated wildcard folder mapping ([#23256](facebook/react#23256)) //<Andrew Clark>// - **[274b9fb16](facebook/react@274b9fb16 )**: Remove path resolution from internal forks plugin ([#23255](facebook/react#23255)) //<Andrew Clark>// Changelog: [General][Changed] - React Native sync for revisions a3bde79...27b5699 jest_e2e[run_all_tests] Reviewed By: rickhanlonii, kacieb Differential Revision: D34241986 fbshipit-source-id: f6ab62df2a918728864283b4f13201275eb3b8a3

* Remove deprecated folder mapping 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. * Remove umd from exports 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.

Summary: This sync includes the following changes: - **[27b569969](facebook/react@27b569969 )**: Simplify cache pool contexts ([facebook#23280](facebook/react#23280)) //<Andrew Clark>// - **[1fb0d0687](facebook/react@1fb0d0687 )**: [Devtools][Transition Tracing] Add Transition callbacks to createRoot ([facebook#23276](facebook/react#23276)) //<Luna Ruan>// - **[a6987bee7](facebook/react@a6987bee7 )**: add <TracingMarker> component boilerplate ([facebook#23275](facebook/react#23275)) //<Luna Ruan>// - **[796fff548](facebook/react@796fff548 )**: Allow suspending outside a Suspense boundary ([facebook#23267](facebook/react#23267)) //<Andrew Clark>// - **[64223fed8](facebook/react@64223fed8 )**: Fix: Multiple hydration errors in same render ([facebook#23273](facebook/react#23273)) //<Andrew Clark>// - **[efd8f6442](facebook/react@efd8f6442 )**: Resolve default onRecoverableError at root init ([facebook#23264](facebook/react#23264)) //<Andrew Clark>// - **[e0af1aabe](facebook/react@e0af1aabe )**: Fix wrong context argument to `apply` //<Andrew Clark>// - **[9b5e0517b](facebook/react@9b5e0517b )**: Remove deprecated wildcard folder mapping ([facebook#23256](facebook/react#23256)) //<Andrew Clark>// - **[274b9fb16](facebook/react@274b9fb16 )**: Remove path resolution from internal forks plugin ([facebook#23255](facebook/react#23255)) //<Andrew Clark>// Changelog: [General][Changed] - React Native sync for revisions a3bde79...27b5699 jest_e2e[run_all_tests] Reviewed By: rickhanlonii, kacieb Differential Revision: D34241986 fbshipit-source-id: f6ab62df2a918728864283b4f13201275eb3b8a3

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 9, 2022

acdlite changed the title ~~Remove deprecated wildcard~~ Remove deprecated wildcard folder mapping Feb 9, 2022

acdlite commented Feb 9, 2022

View reviewed changes

acdlite marked this pull request as draft February 9, 2022 01:02

acdlite force-pushed the remove-deprecated-wildcard branch 3 times, most recently from e667998 to 2fb3805 Compare February 9, 2022 02:46

acdlite commented Feb 9, 2022

View reviewed changes

acdlite force-pushed the remove-deprecated-wildcard branch from 2fb3805 to 05dd99b Compare February 9, 2022 02:52

acdlite marked this pull request as ready for review February 9, 2022 03:02

acdlite requested a review from sebmarkbage February 9, 2022 03:02

acdlite force-pushed the remove-deprecated-wildcard branch 3 times, most recently from 4db18bc to ee66a87 Compare February 9, 2022 04:41

sebmarkbage reviewed Feb 9, 2022

View reviewed changes

sebmarkbage approved these changes Feb 9, 2022

View reviewed changes

acdlite force-pushed the remove-deprecated-wildcard branch from ee66a87 to 7cf1e59 Compare February 9, 2022 16:45

acdlite force-pushed the remove-deprecated-wildcard branch from 7cf1e59 to 67bef4b Compare February 9, 2022 17:03

Remove umd from exports

c281dd9

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.

acdlite merged commit 9b5e051 into facebook:main Feb 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove deprecated wildcard folder mapping #23256

Remove deprecated wildcard folder mapping #23256

acdlite commented Feb 9, 2022 •

edited

acdlite Feb 9, 2022

sebmarkbage Feb 9, 2022

acdlite Feb 9, 2022

acdlite Feb 9, 2022

sebmarkbage Feb 9, 2022

sizebot commented Feb 9, 2022 •

edited

sebmarkbage Feb 9, 2022

acdlite Feb 9, 2022

sebmarkbage Feb 9, 2022

acdlite Feb 9, 2022

sebmarkbage left a comment

acdlite commented Feb 9, 2022

Remove deprecated wildcard folder mapping #23256

Remove deprecated wildcard folder mapping #23256

Conversation

acdlite commented Feb 9, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sizebot commented Feb 9, 2022 • edited

Critical size changes

Significant size changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebmarkbage left a comment

Choose a reason for hiding this comment

acdlite commented Feb 9, 2022

acdlite commented Feb 9, 2022 •

edited

sizebot commented Feb 9, 2022 •

edited