Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: require() doesn't support default exports in ES Modules #4742

Closed
fregante opened this issue Apr 19, 2017 · 32 comments
Closed

Bug: require() doesn't support default exports in ES Modules #4742

fregante opened this issue Apr 19, 2017 · 32 comments

Comments

@fregante
Copy link

fregante commented Apr 19, 2017

✅ ➡️ Edit 2020 for Webpack users:

  • always prefer import nowadays. Only use require if the package is not an ES Module.

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

const ofi = require('object-fit-images');

In webpack 1.x, this picks up the CommonJS file defined in main.

In webpack 2.x, this loads the module file and sets ofi = {default: realOfi}, which means that users have to use the unnatural const ofi = require('object-fit-images').default. Example

If the current behavior is a bug, please provide the steps to reproduce.

npm i object-fit-images@3.1.3
npm i webpack@2.4.1 --global
echo 'var ofi = require("object-fit-images"); ofi()' > src.js; webpack src.js errors.js
echo 'var ofi = require("object-fit-images").default; ofi()' > src.js; webpack src.js works.js
echo 'import ofi from "object-fit-images"; ofi()' > src.js; webpack src.js works-esm.js

What is the expected behavior?

import is for ES Modules and it should try to read the module file if it exists. This works correctly.

require is for CJS files and it should only read the main entry point, not the module one. If it does, it should at least flatten default wherever possible.

Please mention other relevant information such as the browser version, Node.js version, webpack version and Operating System.

node 7.8.0
webpack 2.4.1
OSX 10.11.6

@sokra
Copy link
Member

sokra commented Apr 19, 2017

Packages should not export different things from module and main.

The main/module fields in the package.json describe the entrypoint of the package in ESM and CJS. It doesn't depend on the way you require/import it.

If if would depend on it, it would cause duplication i. e. if you require a module in one file and import it in another file.

So in my opinion this is working correctly. But like to hear more opinions...


I know this is bad for package author because this could mean they need to break the API when adding a ESM export. i. e. when previously exporting a function with module.exports = function objectFitImages() {} they need to change their API to exports.objectFitImages = function() {} or default. objectFitImages = require("object-fit-images") -> objectFitImages = require("object-fit-images").objectFitImages or objectFitImages = require("object-fit-images").default.

@fregante
Copy link
Author

fregante commented Apr 19, 2017

Packages should not export different things from module and main.

That makes sense, but in reality neither of those options is ideal. You wouldn't expect anyone to use something like this:

var webpack = require('webpack').default; //<--
webpack(...);
//or
var webpack = require('webpack');
webpack.webpack(...); //<-- almost ok, but awkward

It defeats the purpose of having a separate module file at all since it only complicates things when using default exports. Without module both of these work as expected:

echo 'var ofi = require("object-fit-images"); ofi()' > src.js; webpack src.js dist.js
echo 'import ofi from "object-fit-images"; ofi()' > src.js; webpack src.js dist.js

@fregante
Copy link
Author

fregante commented Apr 19, 2017

If if would depend on it, it would cause duplication i. e. if you require a module in one file and import it in another file.

You're perfectly right here, always read one file regardless of which import/require method is used.

So how about flattening the default export with interopDefault like babel and rollup do?

webpack could automatically change this:

var ofi = require("object-fit-images");
ofi();

into:

var ofi = require("object-fit-images");
ofi = 'default' in ofi ? ofi['default'] : ofi;
ofi();

fregante pushed a commit to fregante/object-fit-images that referenced this issue Apr 19, 2017
@fregante fregante changed the title Bug: require() loads module instead of main Bug: require() doesn't support default exports in ES Modules Apr 19, 2017
@sokra
Copy link
Member

sokra commented Apr 19, 2017

interopDefault happens when importing a CommonJs module. webpack does the same. No other CommonJs impl does something like you proposed and it doesn't make sense. You won't be able to access other exports except the default export.


The purpose of the module field is to offer the same package content with ESM instead of CJS. You are using it wrong if you put different APIs behind the module field. Exports should match.

export default <=> exports.default =
export function test() <=> exports.test = function()
N/A <=> module.exports

@fregante
Copy link
Author

fregante commented Apr 20, 2017

No other CommonJs impl does something like you proposed and it doesn't make sense.

  1. It makes sense to me since both a = require() and import a from mean get the default export ("default" in the classical sense, not how it's described by the ESM spec)

  2. rollup does that when requiring es modules:

    $ cat module.js 
    export default function foo () {}
    
    $ cat requirer.js 
    const test = require('./module.js');
    console.log(test)
    
    $ cat rollup.config.js 
    export default {
      format: 'cjs',
      plugins: [ require('rollup-plugin-commonjs')() ]
    };
    
    $ rollup requirer.js --config | node
    [Function: foo] # this is console.log(test)
  3. rollup does that when bundling exporting modules, export default X becomes module.exports = X, demo

The purpose of the module field is to offer the same package content

I already agreed with that in my previous comment.

What I'm suggesting is the equivalent of interopDefault when requiring ES default imports. Would you ever suggest using this API for your own modules?

const webpack = require('webpack').default;

.default should never be part of an API.


Now we could even agree that the current situation makes perfect technical sense, but this kills interoperability: it either results in poor APIs (.default) or into the unusability of the module field for default-exporting modules.

@tivac
Copy link

tivac commented May 8, 2017

I just ran into this with modular-css after trying to offer named ES2015 exports. Some CSS classnames aren't valid JS identifiers, so they can't be part of the named exports. Now uses will have to use require("./file.css").default if they want access to all exported classnames.

I'm considering reverting the change to use ES2015 exports because of this, it's now a bad experience for end-users. 😓

@fregante
Copy link
Author

fregante commented May 9, 2017

More context: http://2ality.com/2017/01/babel-esm-spec-mode.html#an-es-module-can-only-default-import-commonjs-modules

For library authors: As a workaround, you could:

  • offer a separate package like lodash does (lodash-es), or
  • skip the module field and use a es-specific entry point, like import a from 'yourpkg/es'
  • skip the module field, avoid es modules (my preferred solution for most of my modules)

The first two choices will be explicit and won't depend on tooling (but they'll need to be documented)
The last choice just works with all CommonJS loaders/tools.

trotzig added a commit to civiccc/react-waypoint that referenced this issue Oct 5, 2017
After releasing 7.3.0, we've seen folks run into issues (#221) with the
new `module` field added to package.json. I've tried to read up on how
webpack uses this field, and it looks like it will pick it up by
default. This is tricky because most people will expect a CommonJS
export (i.e. `module.exports = ...`), but now they suddenly have to
import react-waypoints using

  const Waypoint = require('react-waypoint').default;

There's a lengthy issue over at webpack discussing this [1], with a few
proposed solutions at the bottom:

- offer a separate package like lodash does (lodash-es), or
- skip the module field and use a es-specific entry point, like import a from 'yourpkg/es'
- skip the module field, avoid es modules (my preferred solution for most of my modules)

I'm opting for the second point here. We can use the commonjs bundle as
the default, and then if people are adventurous they can point at the es
module directly:

  import Waypoint from 'react-waypoint/build/index.mjs';

We're not really going to the bottom of this issue with this commit. But
I believe it will solve headaches for a lot of applications. For
instance, I just found out that one developer in my team spent time
chasing down a bug where parts of the application wouldn't work.
Waypoints that were imported with `import Waypoint from
'react-waypoint'` were working, but not the ones being imported via
`const Waypoint = require('react-waypoint')` (we have some legacy).

Fixes #221

[1] webpack/webpack#4742
alanshaw added a commit to ipfs/js-ipfs that referenced this issue May 24, 2018
Big.js just released an update that added esmodule support by adding a "module" field to their package.json. By default, webpack will use this field when requiring a module. Since esmodules aren't fully supported in the browser and we don't do any transpiling this PR configures webpack to not consider the esmodule field.

[Webpack `mainFields` docs](https://webpack.js.org/configuration/resolve/#resolve-mainfields)

[Why `require('big.js')` is broken for us](webpack/webpack#4742)

Fixes #1363

License: MIT
Signed-off-by: Alan Shaw <alan@tableflip.io>
alanshaw added a commit to ipfs-inactive/js-ipfs-http-client that referenced this issue May 24, 2018
Big.js just released an update that added esmodule support by adding a "module" field to their package.json. By default, webpack will use this field when requiring a module. Since esmodules aren't fully supported in the browser and we don't do any transpiling this PR configures webpack to not consider the esmodule field.

[Webpack `mainFields` docs](https://webpack.js.org/configuration/resolve/#resolve-mainfields)

[Why `require('big.js')` is broken for us](webpack/webpack#4742)

Refs ipfs/js-ipfs#1363

Note that this solves the issue for `big.js`, but also for any other modules in the future that decide to define a `module` field in their `package.json` or any that already do and we just haven't realised it yet!

License: MIT
Signed-off-by: Alan Shaw <alan@tableflip.io>
@UziTech UziTech mentioned this issue Dec 8, 2019
12 tasks
coyoteecd added a commit to coyoteecd/typeorm that referenced this issue Jan 8, 2020
…typeorm#4788)

TypeORM uses require() within PlatformTools to load driver packages.
The typeorm-aurora-data-api-driver is exported with export default and packaged with rollup, which, as per webpack/webpack#4742,
causes the require'd module to be available via require('typeorm-aurora-data-api-driver').default property.
pleerock pushed a commit to typeorm/typeorm that referenced this issue Jan 22, 2020
…#4788) (#5302)

TypeORM uses require() within PlatformTools to load driver packages.
The typeorm-aurora-data-api-driver is exported with export default and packaged with rollup, which, as per webpack/webpack#4742,
causes the require'd module to be available via require('typeorm-aurora-data-api-driver').default property.
@fregante fregante closed this as completed Oct 8, 2020
@lygstate
Copy link

anyway to fixes this in webpack???
too much packages have such issues, broken again and again.

@lygstate
Copy link

There is objection, https://www.bignerdranch.com/blog/default-exports-or-named-exports-why-not-both/
I am bitten by these things.

maybe webpack better to provide a optionMap to config each node module are default export or named export.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests