Remove 'import' from default Package Exports conditions

[huntie]

Summary:
The Exports RFC 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.

This assumption is similar to --moduleResolution bundler in TypeScript 5.0:

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:
  • 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 will require future extended behaviour in order to dynamically set 'import' based on how the module is imported/require()d. For now this is not in scope.

Changelog:
[General][Changed] - Default condition set for experimental Package Exports is now ['require', 'react-native']

Metro changelog: [Experimental] Package Exports unstable_conditionNames now defaults to ['require']

Differential Revision: D44303559

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D44303559

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,520,044	+0
android	hermes	armeabi-v7a	7,835,942	+0
android	hermes	x86	8,999,858	+0
android	hermes	x86_64	8,855,178	+0
android	jsc	arm64-v8a	9,141,421	+0
android	jsc	armeabi-v7a	8,333,361	+0
android	jsc	x86	9,195,916	+0
android	jsc	x86_64	9,454,039	+0

Base commit: 0443c2a
Branch: main

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D44303559

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D44303559

[facebook-github-bot]

This pull request has been merged in 308838c.

[huntie]

Counterexample for this is packages which (somewhat incorrectly) specify "import" but not a "default" condition match 🫤: https://github.com/ipfs/js-ipfs-unixfs/blob/master/packages/ipfs-unixfs-importer/package.json. These cases won't work better out-of-the-box.

[robhogan]

That’s sort of unfortunate - but that’s an example of a package which only distributes ESM (see index.js), so there is simply no way to require it - Node JS won’t let you either. There could legally be top level await in there, etc - it’s proper ESM (and the fact that it’s set up that way might be an indication that the authors don’t want to be constrained by CJS).

Node would let you await import() it from CommonJS, so to complete* the work here we ought to allow that from Metro - in that case, the conditions would include import but not require, and we’d be able to resolve it.

[*] - To complete the resolver work, that is - unfortunately there’s a whole can of worms here as to how Metro should deal with ESM. We can (do) YOLO transform it to CJS and turn all its imports into requires, which isn’t semantically sound and will break on ESM->ESM dependencies like this one. Not transforming it isn’t an option because Hermes doesn’t support ESM. There might be a pragmatic “as close to correct as possible” path we can walk here but I’m not sure exactly where that is. Such is the state of ESM in JS today. One to chew over/discuss on Tuesday!