resolver.unstable_enablePackageExports results in exception around interopRequireDefault

[deodad]

New Version

0.72.RC-3

Old Version

na

Build Target(s)

io sim

Output of react-native info

yarn run v1.22.19
$ /Users/deodad/Repos/RN0720RC32/node_modules/.bin/react-native info
info Fetching system and libraries information...
System:
  OS: macOS 13.3.1
  CPU: (12) arm64 Apple M2 Pro
  Memory: 1.04 GB / 32.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 18.14.0
    path: /var/folders/1p/8plc0yx167q53qdktkcfd20m0000gn/T/yarn--1683918327548-0.8468867441091508/node
  Yarn:
    version: 1.22.19
    path: /var/folders/1p/8plc0yx167q53qdktkcfd20m0000gn/T/yarn--1683918327548-0.8468867441091508/yarn
  npm:
    version: 9.6.3
    path: ~/Library/Caches/fnm_multishells/15355_1683908029786/bin/npm
  Watchman: Not Found
Managers:
  CocoaPods:
    version: 1.12.1
    path: /var/folders/1p/8plc0yx167q53qdktkcfd20m0000gn/T/frum_15377_1683908029799/bin/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 22.4
      - iOS 16.4
      - macOS 13.3
      - tvOS 16.4
      - watchOS 9.4
  Android SDK: Not Found
IDEs:
  Android Studio: Not Found
  Xcode:
    version: 14.3/14E222b
    path: /usr/bin/xcodebuild
Languages:
  Java: Not Found
  Ruby:
    version: 3.1.3
    path: /var/folders/1p/8plc0yx167q53qdktkcfd20m0000gn/T/frum_15377_1683908029799/bin/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.72.0-rc.3
    wanted: 0.72.0-rc.3
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: true
  newArchEnabled: false

Done in 0.97s.

Issue and Reproduction Steps

npx react-native@latest init RN0720RC3 --version 0.72.0-rc.3
set resolver.unstable_enablePackageExports true in metro.config.js
yarn start

Unhandled JS Exception: _$$_REQUIRE(_dependencyMap[0], "(...)/helpers/interopRequireDefault") is not a function (it is Object)

TypeError: _$$_REQUIRE(_dependencyMap[0], "(...)/helpers/interopRequireDefault") is not a function (it is Object)
    at anonymous (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:25075:104)
    at loadModuleImplementation (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:328:14)
    at guardedLoadModule (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:227:38)
    at metroRequire (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:123:92)
    at anonymous (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:25052:108)
    at loadModuleImplementation (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:328:14)
    at guardedLoadModule (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:227:38)
    at metroRequire (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:123:92)
    at anonymous (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:24992:14)
    at loadModuleImplementation (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:328:14)
    at guardedLoadModule (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:219:47)
    at metroRequire (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:123:92)
    at global (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:124286:4)

anonymous
    index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:25075:104
loadModuleImplementation
    index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:328:14
guardedLoadModule
    index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:227:38
metroRequire
    index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:123:92
anonymous
    index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:25052:108
loadModuleImplementation
    index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:328:14
guardedLoadModule
    index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:227:38
metroRequire
    index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:123:92
anonymous
    index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:24992:14
loadModuleImplementation
    index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:328:14
guardedLoadModule
    index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:219:47
metroRequire
    index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:123:92
global
    index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.RN0720RC32:124286:4

[kelset]

that's because that option doesn't exists?

AFAIK there's only unstable_enablePackageExports , see https://facebook.github.io/metro/docs/package-exports

[cortinico]

that's because that option doesn't exists?
AFAIK there's only unstable_enablePackageExports , see facebook.github.io/metro/docs/package-exports

Correct 👍 closing this one

[deodad]

this was actually a typo when writing the issue, the flag I am seeing this error with is [unstable_enablePackageExports](https://facebook.github.io/metro/docs/configuration/#unstable_enablepackageexports-experimental) not unstable_enablePackageImports.

Updated the issue, please reopen

[kelset]

@deodad can you provide the full metro config file for the repro? I would like to make sure it's not a misconfiguration

[deodad]

sure thing

const {getDefaultConfig, mergeConfig} = require('@react-native/metro-config');

/**
 * Metro configuration
 * https://facebook.github.io/metro/docs/configuration
 *
 * @type {import('metro-config').MetroConfig}
 */
const config = {
  resolver: {
    unstable_enablePackageExports: true,
  },
};

module.exports = mergeConfig(getDefaultConfig(__dirname), config);

[kelset]

thanks. Can confirm that this indeeds happen:

cc @huntie, @motiz88 and @robhogan that's for you folks. Let us know once you have an idea on how to solve.

[robhogan]

@kelset I'll take a look at this tomorrow. Just to check, is that repro on a fresh react-native init project?

[motiz88]

I'm unable to take a close look at the moment, but I suspect this has something to do with e70ceef. My guess is that Babel generates code that assumes the import and require conditions will be asserted precisely for the respective kinds of dependencies, and never both at once like Metro does. So we end up requireing the version intended for use with import, and not doing any ESM interop unwrapping of the result. (Because this is the ESM interop unwrapping function.)

[motiz88]

If my suspicion above is correct: One approach could be to make the resolver assert either the import or require condition dynamically, based on the type of dependency seen in the AST in collectDependencies. This means also adding this distinction to each dependency's key (edge case: import 'a' and require('a') from the same file can resolve to different files!)

In practice we would end up using the require branch everywhere (because Babel lowers ESM imports to require by that point), except under experimentalImportSupport: true (note: used at Meta but not yet supported in OSS). Which means any packages that expose only an import path would always warn/fail under unstable_enablePackageExports: true until we roll out ESM support to OSS... So that's not exactly ideal.

[robhogan]

In practice we would end up using the require branch everywhere (because Babel lowers ESM imports to require by that point)

The problem we'd have here is that effectively in OSS takes us back to facebook/react-native#36584, which was reverted because it breaks compatibility with ESM-only packages.

We've come back a few times to the idea that we should assert import as a last resort or fallback, maybe after default or only if resolution fails - such behaviour is definitely not within the exports spec, but it might be on a migration path.

[kelset]

@kelset Lorenzo Sciandra FTE I'll take a look at this tomorrow. Just to check, is that repro on a fresh react-native init project?

yes

[NickGerleman]

We've come back a few times to the idea that we should assert import as a last resort or fallback, maybe after default or only if resolution fails - such behaviour is definitely not within the exports spec, but it might be on a migration path.

Is the issue that there are some previously working NPM packages which declare a main that is CommonJS, but exports that has only conditions for ESM and no other default?

If that is the issue, one more fallback strategy might be using main field based resolution if:

1. Exports style resolution fails
2. Exports style resolution would pass if we had conditions to allow ESM

[robhogan]

@NickGerleman - Yeah, we could remove the import assertion and fall right back to non-exports resolution if exports resolution fails. It's fiddly though, partly because this is beyond just main/react-native/browser package entry points but also affects subpath resolution - re your condition 2., just because there's an entry in the exports map for a subpath with the import condition asserted, that doesn't necessarily match how it'd be resolved "traditionally", where the subpath is expected to literally match a path on disk (after appending extensions).

Indeed if we've reached the source file by successfully following the exports map, it might reasonably use subpath imports for its own dependencies that don't point to file locations and must be resolved with exports support - particularly if the dependencies are within a multi-package project like Babel. I haven't seen a concrete example of that, to be fair, but I feel like there are edge cases to whichever spec deviation we choose here.

Re the particular issue with interopRequireDefault, it's worth reiterating as @motiz88 pointed out, this is the module that allows const foo = require('foo') to work interchangeably with import foo from 'foo'. Maybe a surgical fix for that particular module is all we really need, and after that we're fine to assert import.

One possibility might simply to downgrade it - the problem export was only introduced in @babel/[email protected] - compare @babel/[email protected]/package.json, where the CJS version takes precedence.

[robhogan]

Another option, though a gnarly one that's probably breaking in some way - set the default property on CJS module exports within our loadModuleImplementation, and disable the interop layer on our commonjs transform with importInterop: 'none'. Bonus: one fewer function call and object allocation on requires, so potentially a runtime performance boost.

[huntie]

Resolved in https://github.com/facebook/metro, shipping today in Metro 0.76.5 and will also be updated in React Native CLI today.