From d558e4a5ea471934b410358908d3ed95a23c05e3 Mon Sep 17 00:00:00 2001 From: Alex Hunt Date: Thu, 23 Mar 2023 05:01:03 -0700 Subject: [PATCH] Remove 'import' from default Package Exports conditions (#955) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Pull Request resolved: https://github.com/facebook/metro/pull/955 X-link: https://github.com/facebook/react-native/pull/36584 Changelog: [General][Changed] - Default condition set for experimental Package Exports is now `['require', 'react-native']` The [Exports RFC](https://github.com/react-native-community/discussions-and-proposals/blob/main/proposals/0534-metro-package-exports-support.md) had assumed that supporting the `"import"` condition was a syntax-only difference, given we are not in a Node.js environment — and so was worthwhile to support for maximal ecosystem compatibility. {F915841105} This assumption is similar to [`--moduleResolution bundler` in TypeScript 5.0](https://github.com/microsoft/TypeScript/pull/51669): > bundlers and runtimes that include a range of Node-like resolution features and ESM syntax, but do not enforce the strict resolution rules that accompany ES modules in Node or in the browser > -- https://github.com/microsoft/TypeScript/pull/51669#issue-1467004047 However, robhogan has rightly pointed out that **we should not do this!** - ESM (once transpiled) is **not** simply a stricter subset of in-scope features supported by CJS. For example, it supports top-level async, which would be breaking at runtime. - We recently made the same change for our Jest environment: - https://github.com/facebook/react-native/commit/681d7f8113d2b5e9d6966255ee6c72b50a7d488a As such, we are erring on the side of correctness and supporting only `['require', 'react-native']` in our defaults. At runtime, all code run by React Native is anticipated to be CommonJS. `"exports"` will instead allow React Native to correctly select the CommonJS versions of modules from all npm packages. Metro changelog: [Experimental] Package Exports `unstable_conditionNames` now defaults to `['require']` Reviewed By: robhogan Differential Revision: D44303559 fbshipit-source-id: 0d7395ad0a44019ef4b2cad0b43d78dcecc5ecd0 --- docs/Configuration.md | 8 ++++---- .../src/__tests__/__snapshots__/loadConfig-test.js.snap | 4 ---- packages/metro-config/src/defaults/index.js | 2 +- packages/metro-resolver/src/__tests__/utils.js | 2 +- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/docs/Configuration.md b/docs/Configuration.md index 4af2091e4..a94d6464e 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -227,7 +227,7 @@ Metro's default resolver processes each of these fields according to the [`brows :::note -When Metro is started via the React Native CLI, `resolverMainFields` defaults to `['react-native', 'browser', 'main']`. +When using React Native, `resolverMainFields` defaults to `['react-native', 'browser', 'main']`. ::: @@ -340,11 +340,11 @@ The set of [condition names](https://nodejs.org/docs/latest-v18.x/api/packages.h Conditions may be any string value and are resolved in the order specified by each package. Node.js documents a number of [community conditions](https://nodejs.org/docs/latest-v18.x/api/packages.html#community-conditions-definitions) which are commonly used by package authors. The `default` condition is always matched. -Defaults to `['import', 'require']`. +Defaults to `['require']`. :::note -When Metro is started via React Native CLI, `conditionNames` defaults to `['import', 'require', 'react-native']`. +When using React Native, `unstable_conditionNames` defaults to `['require', 'react-native']`. ::: @@ -360,7 +360,7 @@ This setting will take effect when [`unstable_enablePackageExports`](#unstable_e The set of additional [condition names](https://nodejs.org/docs/latest-v18.x/api/packages.html#conditional-exports) to dynamically assert by platform (see [`platforms`](#platforms)) when interpreting the [`"exports"` field](https://nodejs.org/docs/latest-v18.x/api/packages.html#exports) in package.json. -Matched conditions are merged with [`unstable_conditionNames`](#unstable-conditionnames) before resolution. With the defaults for both options, the conditions `new Set(['import', 'require', 'browser'])` will be asserted when requesting a `web` bundle, and `new Set(['import', 'require'])` otherwise. Again, these are resolved in the order specified by each package. +Matched conditions are merged with [`unstable_conditionNames`](#unstable-conditionnames) before resolution. With the defaults for both options, the conditions `new Set(['require', 'browser'])` will be asserted when requesting a `web` bundle, and `new Set(['require'])` otherwise. Again, these are resolved in the order specified by each package. Defaults to `‌{ web: ['browser'] }`. diff --git a/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap b/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap index 1146f694d..171e8134b 100644 --- a/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap +++ b/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap @@ -74,7 +74,6 @@ Object { "tsx", ], "unstable_conditionNames": Array [ - "import", "require", ], "unstable_conditionsByPlatform": Object { @@ -251,7 +250,6 @@ Object { "tsx", ], "unstable_conditionNames": Array [ - "import", "require", ], "unstable_conditionsByPlatform": Object { @@ -428,7 +426,6 @@ Object { "tsx", ], "unstable_conditionNames": Array [ - "import", "require", ], "unstable_conditionsByPlatform": Object { @@ -605,7 +602,6 @@ Object { "tsx", ], "unstable_conditionNames": Array [ - "import", "require", ], "unstable_conditionsByPlatform": Object { diff --git a/packages/metro-config/src/defaults/index.js b/packages/metro-config/src/defaults/index.js index 9c279097f..e3fee1eb8 100644 --- a/packages/metro-config/src/defaults/index.js +++ b/packages/metro-config/src/defaults/index.js @@ -49,7 +49,7 @@ const getDefaultValues = (projectRoot: ?string): ConfigT => ({ nodeModulesPaths: [], resolveRequest: null, resolverMainFields: ['browser', 'main'], - unstable_conditionNames: ['import', 'require'], + unstable_conditionNames: ['require'], unstable_conditionsByPlatform: { web: ['browser'], }, diff --git a/packages/metro-resolver/src/__tests__/utils.js b/packages/metro-resolver/src/__tests__/utils.js index b98cfaad3..13ac1ba0b 100644 --- a/packages/metro-resolver/src/__tests__/utils.js +++ b/packages/metro-resolver/src/__tests__/utils.js @@ -45,7 +45,7 @@ export function createResolutionContext( resolveHasteModule: (name: string) => null, resolveHastePackage: (name: string) => null, sourceExts: ['js', 'jsx', 'json', 'ts', 'tsx'], - unstable_conditionNames: ['import', 'require'], + unstable_conditionNames: ['require'], unstable_conditionsByPlatform: { web: ['browser'], },