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

.mjs and ESM interop with module main, and transpiling to CJS #7482

Closed
jquense opened this issue Jun 5, 2018 · 11 comments
Closed

.mjs and ESM interop with module main, and transpiling to CJS #7482

jquense opened this issue Jun 5, 2018 · 11 comments
Labels

Comments

@jquense
Copy link
Contributor

jquense commented Jun 5, 2018

I've recently run into a bit of a frustrating situation trying to provide ESM build of libraries along-side commonjs ones. Ideally the ESM code should always be used by webpack, and allow cherry picking modules (as to not have to rely solely on treeshaking).

Currently, with the strictness of importing none mjs files into .mjs we can't compile the same codebase in a way that works as cjs or esm modules. take the following example:

_MyComponent.mjs

import { polyfill } from 'react-lifecycle-compat';

// use polyfill(Component) etc.

react-lifecycle-compat exposes both a esm build (.js) and a cjs file here (compiled esm -> cjs)

What I want to do is compile MyComponent.mjs with babel to MyComponent.js and ship both files, relying on webpack to resolve the .mjs file before the .js one. However this won't work.

  • react-lifecycle-compat is not .mjs so webpack considers it not an esm file and requires i write the import as: import ReactLifeCycleCompat from 'react-lifecycle-compat';,
  • Compiling this to cjs will also break, because of react-lifecycle-compat's __esmodule flag, so I can't import that as a default because babel sees taht it's only named exports.

TL: DR;

Consuming compiled, __esmodule flagged, modules with name exports cannot be written in a way that satisfies both cjs and mjs consumers.

I realize I can configure webpack to treat mjs as javascript/auto but that's not ideal, because I'm interested in providing libraries that can be consumed by webpack and node witthout requiring users to configure something, while also wanting to move towards future standards and take advantage of esm optimizations

@sokra
Copy link
Member

sokra commented Jun 5, 2018

Note that .mjs and ESM in .js behaves different. .mjs tries to be as compatible as possible to node.js. This means no module field and no __esModule for .mjs files. Also note that .mjs is still experimental until node.js support has finalized.

Currently to write a package containing ESM and CJS that wants to be compatible to the current state of .mjs you need to do the following:

  • Set main in package.json to a module without extension: i. e. "main": "./index"
  • Create both index.js and index.mjs.
  • (May set "module": "./index.mjs", for bundlers without mjs support)

@jquense
Copy link
Contributor Author

jquense commented Jun 5, 2018

Currently to write a package containing ESM and CJS that wants to be compatible to the current state of .mjs

I don't think that addresses the above issue. I understand how to make my library provide both, It's currently not possible to make a library that contains both types built from the same source, for anything that imports a named export. This repo demonstrates the problem. https://github.com/jquense/mjs-compile-behavior the lib directory has the desired output but index.js is broken because it's using the default import (to make index.mjs happy)

This means no module field and no __esModule for .mjs files.

I understand that, and totally get why, but the downside here is that it limits library authors ability to even adopt mjs because it's not possible to provide both cjs and mjs builds without maintaining seperate sources, for each target. Which is not feasible of course.

I'm not necessarily arguing that the current behavior is a bug, but it definately is not ideal, and prevent's common use-cases. I think generally we want libraries to more easily adopt the standard, but the current behavior makes it nearly impossible.

@sokra
Copy link
Member

sokra commented Jun 5, 2018

That's how it's spec'ed. You should tell your concerns to the node.js team. We only mirror the node.js ESM implementation.

@jquense
Copy link
Contributor Author

jquense commented Jun 5, 2018

I get that, but there is zero chance that node will care. This is an issue that faces folks using bundlers. If it i only had to run the code in node it wouldn't be a problem i'd just use common js. The only reason we'd use ESM here, is that webpack provides optimizations.

I really do understand that it's not to spec, but

  • importing named exports from CJS It not settled in node (if i understand @jdalton correctly)
  • The web has different concerns, node doesn't have to worry about this, and this behavior actively discourages folks from switching to mjs on the web side.
  • A lot of webpacks behavior around module resolution is not to spec, nor does it match node, require.ensure, magic import() comments, the ability to configure webpack to do whatever you want...

I'm only trying to make a case for the situation that (i think) encourages folks to actually get on board with .mjs on the web side. It's not gonna matter if the behavior matches node if there is no reason to use it in the first place.

@jquense
Copy link
Contributor Author

jquense commented Jun 5, 2018

sorry if i sound frustrated :)

I just wish i could spent 90% of my professional life trying to make modules work together for folks :P

@jdalton
Copy link
Contributor

jdalton commented Jun 5, 2018

Super simple solution is to not use .mjs for your bundles if you want the existing ecosystem interop. If you use .js for your ESM code to be bundled then named exports will continue to work without any extra confinging I believe.

@jquense
Copy link
Contributor Author

jquense commented Jun 5, 2018

If you use .js for your ESM code to be bundled then named exports will continue to work without any extra confining I believe.

yes but we lose support for cherry-picking from those packages.. webpack will resolve .mjs files before .js ones so it "just works" aside from the above problems. Not cherry-picking isn't great for cjs modules in a browser.

I find it really hard to outline the actual problems simply since they only really show up across libraries requiring libraries...

  • lib react-bootstrap imports react-overlays/lib/Overlay
  • my app imports react-overlays

^-- results in esm build and cjs Overlay (and deps) included in a bundle

  • lib react-bootstrap imports react-overlays
  • my app imports react-overlays and react-bootstrap

^--- works maybe with treeshaking in a webpack context, but a CJS context gets everything. not to mention treeshaking is finicky and easy mess up

@jdalton
Copy link
Contributor

jdalton commented Jun 5, 2018

Ah, so this is because of third-party deps which use .mjs. They really shouldn't be jumping on board that soon either. However, you can override the webpack config for this...

{
  type: 'javascript/auto',
  test: /\.mjs$/,
  use: [ ...loaders ]
}

@sokra
Copy link
Member

sokra commented Jun 6, 2018

  • lib react-bootstrap imports react-overlays/lib/Overlay
  • my app imports react-overlays

^-- results in esm build and cjs Overlay (and deps) included in a bundle

No neccessary. Publish "react-overlays" this way:

  • package.json ("main": "./lib/index" "module": "./lib/index.mjs")
  • lib/index.mjs <- original ESM source
  • lib/index.js <- babel transpiled
  • lib/Overlay.mjs <- original ESM source
  • lib/Overlay.js <- babel transpiled
  • ...

react-bootstrap importing react-overlays/lib/Overlay will resolve to lib/Overlay.mjs, including the ESM source.

Your app importing react-overlays will resolve to lib/index.mjs, including the ESM source.

No CJS source is included.


Assuming a bundler not supporting .mjs: It will pick up the CJS source. The module field doesn't support cherry-picking so including the module field would actually cause duplicates for bundlers which only support the module field but not .mjs. Best add a note to add .mjs to extensions for those bundlers.

@webpack-bot
Copy link
Contributor

This issue had no activity for at least half a year.

It's subject to automatic issue closing if there is no activity in the next 15 days.

jquense referenced this issue in formatjs/formatjs-old Aug 8, 2019
@webpack-bot
Copy link
Contributor

Issue was closed because of inactivity.

If you think this is still a valid issue, please file a new issue with additional information.

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

No branches or pull requests

4 participants