require() prefers ES Modules?

[tmcw]

Is this a bug report?

Yes, I think this is a bug report.

Did you try recovering your dependencies?

Yes, and this bug doesn't have anything to do with installing dependencies.

Which terms did you search for in User Guide?

• ES Modules
• require
• CommonJS

Environment

➜  cra git:(master) ✗ npx create-react-app --info
npx: installed 63 in 1.784s

Environment Info:

  System:
    OS: macOS 10.14.2
    CPU: x64 Intel(R) Core(TM) i7-8559U CPU @ 2.70GHz
  Binaries:
    Node: 10.14.1 - ~/n/bin/node
    Yarn: 1.9.4 - ~/.yarn/bin/yarn
    npm: 6.7.0 - /usr/local/bin/npm
  Browsers:
    Firefox: 64.0.2
    Safari: 12.0.2
  npmPackages:
    react: ^16.7.0 => 16.7.0
    react-dom: ^16.7.0 => 16.7.0
    react-scripts: 2.0.3 => 2.0.3
  npmGlobalPackages:
    create-react-app: Not Found

Steps to Reproduce

(Write your steps here:)

1. Write a module that you want people to use, providing it as a CommonJS module.
2. Try to modernize the module, and provide an ES Module entry point, exposing it under module. This feels, to you, like a minor change: you're adding something.
3. Your users use the module with create-react-app and require(). require(), for some reason, loads the ES Module now, not the CommonJS module, and in the unfortunate haphazard way that the two interop, it is now 'broken', and the require() returns an object with {default:fn()} instead of fn() as it did before.

Expected Behavior

I assumed that the CommonJS syntax require() would use the CommonJS entry point, and the ES Module import would use the ES Module entry point, and that this wouldn't be a breaking change.

Actual Behavior

The module appears to be 'broken' to people consuming it via create-react-app - and possibly all similar webpack-related toolchains.

As a module maintainer, this leaves me with no clear way to introduce ES Module entry points into my modules without breaking them for CommonJS users. And I can't see why this behavior - apparently preferring the module entry point to the main one even through require() would be default.

Reproducible Demo

One-commit demo is at https://github.com/tmcw/cra-example

The one commit is https://github.com/tmcw/cra-example/commit/c1262372fba3104f25d6dad3d77c37b0ad8a4442 which adds a dependency, uses it with require() and console.logs it.

[mrmckeb]

Hi @tmcw. This is actually the way the Webpack 4 is handling modules, not us. If ES modules are supplied, they're preferred - which is great news, as it should cut down on bundle size.

For you, as a maintainer or publisher, I get that it's confusing. Users, when using require, will need to require(...).default. This wouldn't be an issue if they were importing using import.

Let me know if you have any other questions, and sorry that this has caused you issues.

[tmcw]

Thanks for the detail. Well, that's a bummer. In my case, it looks like

[someone's application] → depends on [module A i don't control] → depends on [my module B]

So, module A specified a semver range for its dependency on B, and uses require(). So, when I publish a patch or minor update that adds an ES module entry point to module B, it breaks module A, and then breaks someone's applications. I know this isn't the void to be shouting into, but geez that's not a good system.

Module A might not want to publish an ES Module entry point, or it might be abandoned or unmaintained - in this case, it is unmaintained. This puts additional strain on module authors and makes a backwards-compatible addition non-backwards-compatible only for webpack users.

[ghost]

If ES modules are supplied, they're preferred - which is great news, as it should cut down on bundle size.

? tree-shaking works fine on commonjs modules too using https://github.com/indutny/common-shake
The zeal for ES modules without enough regard for interop and backwards compat has caused so many problems, like this one.

[mrmckeb]

Hi @tmcw, that's very interesting... I don't know, but I would assume that a dependency of a dependency shouldn't be handled that way - for the exact reason you've highlighted. Perhaps this is an issue to discuss with the Webpack team?

Thanks @substack, I was talking about out-of-the-box webpack though.

[tmcw]

Hi there,

I went digging to find a core webpack bug to report, but unfortunately I found my way back here. Here's a minimal testcase, based on an ejected create-react-app configuration: https://github.com/tmcw/create-react-app-6321

I've confirmed that this behavior doesn't happen with an zero-config Webpack 4 setup and is only triggered by create-react-apps's overeager (imho) transpilation of all app dependencies and sub-dependencies.

[mrmckeb]

Thanks @tmcw, this will obviously need more investigation then. Would you be interested in digging a little deeper and raising a PR? If not, I could try to get onto it over the weekend.

[davidgilbertson]

@tmcw there's a webpack issue here webpack/webpack#5756

From 2017.

@sokra said "won't fix"

And there's another one from 2018 that's pretty much the same thing with the same response: webpack/webpack#6584

[sokra]

Using different main fields for require() and import would lead to duplicate packages included in your bundle.

Let's assume react would have source code in ESM and CJS and include module and main in package.json.

One package i. e. react-super-dropdown has require("react") (or import "react", but transpiled to CJS via babel). Another package, i. e. react-mega-layouthasimport "react"`.

In this case react would be included twice in the package, because react-super-dropdown will follow the main field to react/index.js and react-mega-layout will follow the module field to react/esm/index.js.

This is bad for bundle size, in this reacts case it even breaks the app. In this case you would have to ensure to only use CJS or ESM packages. Probably resulting in only CJS, which will lead to complete banning of ESM, which would make everyone very sad.

Yes, always preferring the module field cases some issues when packages export different API for ESM and CJS, but the other solutions would even cause more issues...

My guideline for package authors:
Make sure to expose the same API for ESM and CJS.
Note that if your CJS lib currently exposes only one thing via module.exports = you can't add ESM without a breaking change. Change the API to exports.Something = and it will be possible to add ESM.

[davidgilbertson]

Ah, I hadn't considered the scenario where two packages import the same package differently.

So it seems there's no two ways about it: package authors will one-by-one add a modules property to their package.json, that will break a bunch of other packages in the process, and those packages will get GitHub issues and around and around we go :(

I wonder, where on the internet is the suggestion to add the non-standard modules property? Should that info come with a warning?

[tmcw]

To the extent that anything beyond what npm documents in package.json is 'standard', the 'module' field is a community standard and has de-facto documentation in a number of locations - a spec proposal, bundler docs, etc.

Tobias's rationale makes sense, even though it's a little inconvenient for me 😉. But the issues that remain are that:

• It's too late for modules to avoid using module.exports =. Tons of them do.
• This danger doesn't seem to be mentioned anywhere other than bug reports, so it was a surprise to me, and according to the mentions in the webpack issue tracker, was a surprise to many others, leading package authors to bear a lot of the feedback. It certainly never occurred to me that adding a module field would constitute a breaking change.
• The breakage is hard to track down - there's no explicit error message, and code that breaks this way will break in production or test, not in the compile step.

Edit: I'm happy to write up a PSA about "adding module is (probably) a major version change"

[mrmckeb]

I agree that this is a little messy right now (as an industry) and we need to keep working together to improve the situation.

However, I feel that perhaps this is a good time to close this thread off unless anyone has any suggestions on what we can do to make this easier for future developers?

[tmcw]

Yep, besides spreading the knowledge I don't see any other tasks here.