ai/nanoevents

Importing error in React Native starting from nanoevents 3.0 version

farwayer opened this issue · 18 comments

Because .cjs source extension is not very commonly used and React Native do not know it and interprets as part of filename.

error: bundling failed: Error: While trying to resolve module `nanoevents` from file `.../src/api/ws/client.js`, the package `.../node_modules/nanoevents/package.json` was successfully found. However, this package itself specifies a `main` module field that could not be resolved (`.../node_modules/nanoevents/index.cjs`. Indeed, none of these files exist:

  * .../node_modules/nanoevents/index.cjs(.native|.android.js|.native.js|.js|.android.json|.native.json|.json|.android.ts|.native.ts|.ts|.android.tsx|.native.tsx|.tsx)
  * .../node_modules/nanoevents/index.cjs/index(.native|.android.js|.native.js|.js|.android.json|.native.json|.json|.android.ts|.native.ts|.ts|.android.tsx|.native.tsx|.tsx)

Workarounds:

  1. import {createNanoEvents} from 'nanoevents/index'
  2. add .cjs extension to metro.config.js:
const {getDefaultValues} = require('metro-config/src/defaults')
const {resolver: {sourceExts}} = getDefaultValues()

module.exports = {
  transformer: {
    getTransformOptions: async () => ({
      transform: {
        experimentalImportSupport: false,
        inlineRequires: false,
      },
    }),
  },
  resolver: {
    sourceExts: [...sourceExts, 'cjs'],
  },
}

But it will be nice nanoevents package do not cause the error and works without need an additional configuration.

  1. Use index.cjs.js file name instead
  2. Or add react-native field to package.json with index.js value

This extension is part of node resolution algorithm and a requirement for conditional exports (native node es modules support). The best solution is to open the issue in metro bundler to support it out of the box.

Alternatively you can specify to prefer "module" entry point over "main". Though idk if it's possible in metro.

As I can see type, main, module and exports fields are already defined in package.json so nodejs already knows all necessary info for importing package in any mode even without using .cjs extension.

nanoevents is first package I ran into this problem with React Native. Does all other packagers know about .cjs? If it is standard extension than of course it is better to add support of this to metro bundler. But if it is Nodejs only then maybe it is better to use more default .js to prevent such situations IMO.

If type is "commonjs" then all es modules should have mjs extension. If type is "module" then all commonjs modules should have cjs extension. This is how dual mode works in node. Module types have to be distinct somehow. It's the first package because only a few migrated yet. Node removed experimental flag only a few weeks ago.

I mean it's technical requirement not just fancy feature.

ai commented

Use index.cjs.js file name instead

We need .cjs name, because otherwise, Node.js will load it as ES module.

nanoevents is first package I ran into this problem with React Native. Does all other packagers know about .cjs?

Most of the npm packages do not provide ESM for Node.js. Most of the packages provide ESM for webpack only, which does not follow the spec.

If it is standard extension than of course it is better to add support of this to metro bundler.

Yeap, it is part of Node.js standard.

Because .cjs source extension is not very commonly used and React Native do not know it and interprets as part of filename.

Yes, we should start by creating an issue in the metro builder. Can you do it since I am less familiar with the React Native ecosystem?

I do not like “it is your tool” problem answer in the issue. You have a real problem and we need a solution.

Right now I see this solution:

  1. Put ESM file to index.mjs instead of index.js
  2. Put CJS files to index.js instead of index.cjs
  3. Remove type: 'module' from package.json
  4. Use Conditional Exports to load index.mjs on require('nanoevents/index.js').

Unfortunately, we need to support all kind of ESM environments:

  1. Node.js
  2. Webpack, Parcel, Rollup
  3. Loading ESM files to the browsers without bundler by jsDelivr

Unfortunately the index.js → index.mjs fix will force all CDN users to manually add index.mjs to the URLs. I will think about the better solution today and, if I will not find the best solution, I will think about who we should suffer in the situation.

ai commented

Another idea is to create separated index.native.js for each file and use them instead of index.cjs by package.react-native directive.

ai commented

I opened an issue in Metro builder

facebook/metro#535

ai commented

@farwayer can you try Nano Events 5.1.1 with this config?

Wow, thank you a lot Andrew for fast answer and fixes. I do not like “it is your tool” problem answer too and try to make everything work out of the box for users. JS ecosystem is already very difficult for developers to add even more complexity.

Some suggestions:

  1. Metro supports ESM modules so you can use it instead CJS.
  2. You do not need extra js file, nor custom metro config at all. All you need is to add react-native field with path to ESM index file. It is enough.

One more small issue I trap into is warning while starting metro server:

warn Package nanoevents has been ignored because it contains invalid configuration. Reason: Package subpath './package.json' is not defined by "exports" in project/node_modules/nanoevents/package.json

Look like metro trying to import nanoevents/package.json file internally and can't find it in exports. But everything works without problems.

@ai fix "csj" in metro issue

ai commented

If I send ESM files to Metro I got

Error: Export statement may only appear at top level in file node_modules/lib/a/index.js at 9:2

lib/a/index.js is:

function a () {
  console.log('cjs a')
}

let toShare = 'shaked-export'

export { a, toShare }

Here is the test

@farwayer

  1. Can you try new Nano Events 5.1.2 fix for package.json warnings?
  2. Can you help me with export error described above? Maybe it is just bug in my test environment
  1. Warning is gone
  2. ai/dual-publish#2
ai commented

@farwayer I found that you PR didn’t fix the problem

New metro doesn’t load pkg['react-native] anymore. With your fix React Native still goes to pkg.main and looks for index.cjs. Your fix works because RN resolves that index.cjs to index.cjs.js.

But at least it does not require to change resolverMainFields.

Here is dual-publish test fix, that shows, that React Native just transpiles import/export` calls by Babel to CommonJS and has no ESM support. ai/dual-publish@0ff0d7d

ai commented

I released new Nano Events 5.1.3 which works in React Native without any config (thanks for the help).

Unfortunately, without .cjs support fix from Metro we must have 3 files: index.js (for webpack, Node.js ESM, and browsers), index.cjs (for Node.js CJS), and index.cjs.js (for React Native). I think it is OK for temporary solution.

If you will leave a comment in my Metro issue with maintainer mention—it could help move from temporary solution faster.

New metro doesn’t load pkg['react-native'] anymore. With your fix React Native still goes to pkg.main and looks for index.cjs.

@ai the problem was not with pkg['react-native'] field but with test configuration. Field in package works in React Native! But it is configured by react-native cli https://github.com/react-native-community/cli/blob/master/packages/cli/src/tools/loadMetroConfig.ts#L53. My bad don't know about it. So please add

resolver: {
  resolverMainFields: ['react-native', 'browser', 'main']
}

to test (as default configuration for React Native). You can back pkg['react-native'] and React Native will use ESM build.

ai commented

@farwayer thanks for fact that React Native overwrite resolverMainFields.

I simplify React Native support with native ESM and without extra files ai/dual-publish@9d8b073

New Dual Publish and Nano Events versions were published.

@ai @farwayer @TrySound I would be curious to learn from you if you think package.json should be included in all pkg.exports fields or whether this is a "bug" in react-native: react-native-community/cli#1168