Skip to content

Commit

Permalink
Remove 'import' from default Package Exports conditions (#955)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #955

X-link: facebook/react-native#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](microsoft/TypeScript#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
> -- microsoft/TypeScript#51669 (comment)

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:
    - facebook/react-native@681d7f8

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: 0077e547e7775e53d1e4e9c3a9d01347f4fb7d4a
  • Loading branch information
huntie authored and facebook-github-bot committed Mar 23, 2023
1 parent 5ff1cf9 commit 0b2e7a7
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 10 deletions.
8 changes: 4 additions & 4 deletions docs/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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']`.
:::
Expand Down Expand Up @@ -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']`.
:::
Expand All @@ -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'] }`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ Object {
"tsx",
],
"unstable_conditionNames": Array [
"import",
"require",
],
"unstable_conditionsByPlatform": Object {
Expand Down Expand Up @@ -251,7 +250,6 @@ Object {
"tsx",
],
"unstable_conditionNames": Array [
"import",
"require",
],
"unstable_conditionsByPlatform": Object {
Expand Down Expand Up @@ -428,7 +426,6 @@ Object {
"tsx",
],
"unstable_conditionNames": Array [
"import",
"require",
],
"unstable_conditionsByPlatform": Object {
Expand Down Expand Up @@ -605,7 +602,6 @@ Object {
"tsx",
],
"unstable_conditionNames": Array [
"import",
"require",
],
"unstable_conditionsByPlatform": Object {
Expand Down
2 changes: 1 addition & 1 deletion packages/metro-config/src/defaults/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
},
Expand Down
2 changes: 1 addition & 1 deletion packages/metro-resolver/src/__tests__/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
},
Expand Down

0 comments on commit 0b2e7a7

Please sign in to comment.