babel / babel

🐠 Babel is a compiler for writing next generation JavaScript.
https://babel.dev
MIT License
42.99k stars 5.59k forks source link

Modules interop #95

Closed guybedford closed 9 years ago

guybedford commented 9 years ago

I thought it may be time to discuss multi-format module interop. This is a difficult discussion, and most discussions about this tend to reach a stalemate, since you have to go quite far down a road of thought to see where it falls over, so it can be tricky to switch between viewpoints in a single discussion. I'm starting it off entirely from my viewpoint, which is particularly based around the ES6 module loader, so do be wary of my biases too. There is mixed agreement about this stuff, so I'll try to give as balanced a view as I can.

Consider if I have an application which has native CommonJS modules and CommonJS modules generated from ES6, and I want those two to play well together.

An example scenario is:

es6-main.js

import p from 'cjs';
import { q } from 'es6';
export function r() {}

cjs.js

module.exports = 'cjs';

es6.js

export var q = 'es6';

Where es6.js and es6-main.js need to be transpiled to CommonJS that will work with cjs.js after transpilation.

The output we do in Traceur currently is something like (a slightly more readable version here):

es6.js ->

exports.q = 'es6';
exports.__esModule = true;

es6-main.js ->

var cjsModule = require('cjs');
if (!cjsModule || !cjsModule.__esModule)
  cjsModule = { default: cjsModule }; // treat non-ES6 as default-only exports
var p = cjsModule.default;

var es6Module = require('es6');
if (!es6Module || !es6Module.__esModule)
  es6Module = { default: es6Module }; // this won't pass for the ES6 module

exports.r = function r() {}
exports.__esModule = true;

Note that this is a case of unlinked interop compilation, where we only consider how to compile on an individual file basis, without doing any deeper lookups into the tree at compilation. If we did look at the tree more closely, we could remove the second conditional, since we know it is ES6. It would be possible to write a resolver that can deduce this info at compile time, but still be flexible to allow modules that don't resolve at compile time.

The basic assumption of unlinked interop is that we are trying to compile so that imports should work against other module formats, without having to assume in advance how those imports resolve. The reason I like this scenario is because it maintains module portability. I can publish to npm, and perhaps tomorrow the package at cjs.js gets updated on npm to be based on ES6 code. Similarly, I can run my code in an ES6 module loader as ES6, and it still behaves like the CommonJS-compiled version did.

If we don't think about the above, and we continue doing what we are doing currently here (sorry I didn't want to go into all this before), which is:

import * as p from 'cjs';
import { q } from 'es6';

Then if I try to load the above in an ES6 module loader, it will load cjs.js and create a new module object for it, Module({ default: 'cjs' }). We thus get the wrong thing when loading the ES6 module.

The point is, code written this way does not support non-object CJS exports, and will not easily run in an ES6 module loader

The reason is that we have to treat CommonJS modules as a default export only in an ES6 module loader since they can be any object type and module objects in the loader registry must be plain namespaces.

The pain point here for consumers is that they want to be able to write:

import { readFile } from 'fs';

And have that work against existing NodeJS modules. But interop and compiler concerns do trump this in my opinion. We can add an addition to make that work here, and that is to extend a CommonJS module and iterate the exported names and decorate the namespace with them: Module(extend(getAllProperties(fs), { default: fs }). This way it may still be possible to export pieces off of a CommonJS object like above. This could potentially be a middle ground and is currently a point of debate. My personal opinion is against this type of work, but a lot of others are for it.

Note also that the exact rules and choices in this process entirely depend on the linking assumptions of the compiler and runtime assumptions of the code. My bias is towards unlinked portable single-file static compilation as the most general case, also running in the dynamic es6 module loader. But it may be interesting to explore the middle grounds as mentioned above where the linked / unlinked boundary is given separate treatment.

The other extreme is to rather look at resolving any imported modules at compile time (CommonJS node lookup / UMD support etc), and use those resolutions to know how to treat the imports. If you do want to allow custom resolution at compile time, this is as simple as a normalize hook into the compiler - normalize(path, parentPath), and is how the ES6 module loader works.

There are lots of long discussions about this stuff, but sometimes its easier to just go through everything again fresh rather so I'm happy to discuss further. In case you are interested - https://github.com/google/traceur-compiler/issues/1388, https://github.com/google/traceur-compiler/issues/1295, https://github.com/esnext/es6-module-transpiler/issues/156, https://github.com/esnext/es6-module-transpiler/issues/86, https://github.com/esnext/es6-module-transpiler/issues/85.

sebmck commented 9 years ago

I'm leaning more towards simply just transforming:

import fs from "fs";

into

var _fs = require("fs");
var fs = _fs.default || _fs;
guybedford commented 9 years ago

Interesting, but what about other named imports?

sebmck commented 9 years ago
import { readFileSync } from "fs";
var _fs = require("fs");
var readFileSync = _fs.readFileSync;
guybedford commented 9 years ago

What about import * as fs from 'fs', where fs could be a non-namespace object like a function? The simple translation would effectively break ES6 in that the module object needs to be a namespace object only.

jamiebuilds commented 9 years ago

where fs could be a non-namespace object like a function?

@guybedford What do you mean by this?

guybedford commented 9 years ago

The issue is the following:

stream.js

function Stream() {
}
// ...etc...
module.exports = Stream;

We're supposed to be importing this with:

import stream from 'stream';

But with the current naive translation the above doesn't work and consumers will rather write:

import * as stream from 'stream';

The above is breaking ES6 because import * is designed to import a special namespace object with readable properties only (https://people.mozilla.org/~jorendorff/es6-draft.html#sec-module-namespace-exotic-objects). The object itself is not supposed to have any meaning - it is just a namespace object providing access to the named exports.

So if users write import * and try to use this in an actual native ES6 modules environment, it won't work as we can't cast stream into a namespace object.

The comprehensive option would be to provide an inline conversion of the module into a namespace object, but this is quite a bit of inline code.

A simple option may be to simply throw in this scenario, and thereby encourage the import stream from 'stream' form.

sebmck commented 9 years ago

I quite like the idea of throwing an error on import *. I'm pretty weary on adding a lot of extra inline code and I don't particularly want to encourage bad practices either.

Although then there's no way to access all exports of a file. How about transforming:

import stream from "stream";

to

var _stream = require("stream");
var stream = stream.default || stream;

and

import * as stream from "stream";

to

var _stream = require("stream");
var stream = typeof _stream === "object" ? _stream : { default: _stream };
jamiebuilds commented 9 years ago

For more than a single module it might be better to do something like this:

var _ref;
var fs = (_ref = require('fs')).default || _ref;
var stream = typeof (_ref = require('stream')) === 'object' ? _ref : { default: _ref };

This will minify slightly better for more than a single module and there are less temporary variables.

guybedford commented 9 years ago

@sebmck that sounds sensible.

The only issue with the above is you've now forked the module object - since in one scenario it has only a default property (import * as stream from 'stream'), while in another it has named exports (import {Readable} from 'stream'). It would be good to ensure consistency here.

If you're open to throwing then perhaps we can stick with your suggested behaviours and just do:

import * as Stream from 'stream';
var Stream = require('stream');
if (!Stream.__esModule) throw new TypeError('import * only applies to ES6 modules');

This means still adding an __esModule with ES6 -> CommonJS conversions as well.

guybedford commented 9 years ago

@sebmck note that the point is that all exports are accessed via import Stream from 'stream' for CommonJS, but import * as ES6 from 'es6' for ES6. So we're just trying to ensure users always make the right choices!

jamiebuilds commented 9 years ago

@guybedford the problem with throwing an error is that it make modules updating to use es6 a breaking change, which they might not realize and do in a minor or patch release.

I do agree that it should be marked with some kind of __esModule property since having a default property does not necessarily mean it's es6.

guybedford commented 9 years ago

@thejameskyle do you mean for the __esModule flag check, if the underlying CommonJS module becomes actual ES6? This is why I've posted three times to es-discuss asking for a Reflect.isModule function to detect real ES6 module object, but no one else has backed me up on this yet.

See my spec issue here - https://bugs.ecmascript.org/show_bug.cgi?id=3250.

Note that we could ammend the output to be:

var Stream = require('stream');
if (!Stream.__esModule && !(typeof Stream == 'object' && Stream.toString() == 'Module')) throw new TypeError('import * only applies to ES6 modules');

To allow the output to work alongside real ES6 modules in ES6 environments for the full reverse interop, assuming this is what you are referring to!

Alternatively, back me up on the spec issue if you want something simpler!

jamiebuilds commented 9 years ago

I was actually thinking about this some more, and I don't think adding __esModule is going to be all that great either because it locks people into using 6to5.

I'd like to have a Reflect.isModule method, however we need something that's going to work across es6 implementation today.

guybedford commented 9 years ago

__esModule is supported by Traceur and es6-module-transpiler has expressed willingness for implementation - https://github.com/esnext/es6-module-transpiler/issues/156#issuecomment-60163481.

Interop only works through coordination.

jamiebuilds commented 9 years ago

Interop only works through coordination.

Agreed, it'd be good to coordinate with @arv and @eventualbuddha on making this all work together to push es6 modules forward. I'd like to see how this will work once es6 modules are implemented natively.

arv commented 9 years ago

@johnjbarton is the Traceur module expert.

jamiebuilds commented 9 years ago

Sorry @arv I was just grabbing the top contributors for each project. @johnjbarton thoughts?

johnjbarton commented 9 years ago

(I may be the disruptive force here since I am more interested in long term success. Despite my questions below I'm not against adopting a strategy that comes out of discussions.).

What I don't like about this direction is that it creates more barriers to an ES6 world by encouraging tranpiling ES6 into ES5 node modules and shipping them. I would be more enthusiastic if we had an end game where node loads ES6 modules and npm works great with es6. Then maybe I could see how this interop leads us towards that goal. Maybe you all know how that will be already?

If we had a node loader with the capabilities of es6-module-transpiler and es6-module-loader, that is just enough transcoding to support import and export, then we could have these keywords in the interop module format. es6 code would be transpiled to es5 excepting these module bits. As node gains es6 powers the transpiling fades to nothing.

In other words, I'm just asking if there a strategy that lifts node up to new modules rather than compiling down to old module?

jamiebuilds commented 9 years ago

@johnjbarton It'll be awhile before node modules distribute themselves as primarily es6 modules, simply because doing so would require the user to worry about the implementation.

If the compile step for these modules was extremely straightforward so that either side could be using cjs/6to5/traceur/es-mt and never be concerned with it, then many people will choose to author in es6 with a compile step today, so that in the future all they have to do is remove the compile step.

If a module authors in es6 and it's a headache for cjs users or users need to compile it themselves, it will be a big barrier for adoption.

I am more interested in long term success

I'm willing to bet everyone involved is interested in the long term success of es6 :smile:

johnjbarton commented 9 years ago

Earlier @thejameskyle said:

I'd like to see how this will work once es6 modules are implemented natively.

That is also my question. Is there any answer other than "We'll always use cjs"?

johnjbarton commented 9 years ago

Interop only works through coordination.

I'm trying to figure out what are the open issues here.

  1. Reflect.isModule() vs __esModule? Then how would isModule() be implemented?

all exports are accessed via import Stream from 'stream' for CommonJS, but import * as ES6 from 'es6' for ES6.

This seems to imply that users know about implementation contrary to the goal. But within that limitation, using var Stream = require('stream'); for CommonJS seems simple and less likely to cause other problems.

3.?

jamiebuilds commented 9 years ago

So shimming it on both imports and exports will probably be necessary. I remember hearing some discussion on creating a UMD that worked with es6 modules awhile back but I can't find it now.

eventualbuddha commented 9 years ago

I'm definitely still open to implementing some interop in the es6-module-transpiler. The approach I've been looking at recently is to use the es6-module-transpiler-npm-resolver, which means shipping npm modules with both ES6 and ES5 code. cc @caridy @ericf

guybedford commented 9 years ago

Thanks @thejameskyle for getting the right people here.

@eventualbuddha @johnjbarton the main issue being discussed here is that, regardless of whether we endorse it or not, the ability to compile ES6 to CommonJS means that users will be able to load normal CommonJS as dependencies. If we don't tackle this interop upfront, we open up to interop issues in future. Note also that this interop of ES6 loading other formats through the module syntax is exactly the interop that is designed to be supported by the module loader.

I'd appreciate it if we can put aside the scenario of loading ES6 and CommonJS in the same file for this discussion, as regardless of the outcome of that discussion, the above issue remains the same. Happy to discuss the details elsewhere anytime.

I do think compiling ES6 to CommonJS as separate files in this way is a great backwards-compatibility and transition vector for NodeJS, so do think this is a scenario that should be encouraged.

So it would be amazing if we can come to agreement on the behaviour of how CommonJS loads through module syntax - specifically what does the module object look like, and how do we ensure consistent interop.

Based on these discussions so far with @sebmck the output I would recommend here for 6to5, based on the project goals of code readability, is the following:

  // named imports work for both CommonJS and ES6
  import {readFile} from 'fs';
  // ->
  var readFile = require('fs').readFile;

  // default imports branch between CommonJS and ES6:
  import fs from 'fs';
  // ->
  var _mRef;
  var fs = (_mRef = require('fs')).__esModule ? _mRef.default : _mRef;

  // module object import gets module for ES6, and just { default } for CommonJS:
  // this is a restriction of behaviour, but a backwards-compatible one with ES6 environments
  import * as fs from 'fs';
  // ->
  var _mRef;
  var fs = (_mRef = require('fs')).__esModule ? _mRef : { default: _mRef };

The above would entirely interoperate with the Traceur output, real ES6 environments and the module loader, with the one provisor as @thejameskyle pointed out, that upgrading one of our CommonJS modules to ES6 in an ES6 environment is now a breaking change since the __esModule flag doesn't work here.

This is where a Reflect.isModule function can be useful - help advocating this at the spec level would be amazing, but without it, we can probably craft something like the following:

// see https://people.mozilla.org/~jorendorff/es6-draft.html#sec-@@tostringtag
function isModule(m) {
  return m.__esModule || typeof m == 'object' && m.toString() == 'Module';
}
guybedford commented 9 years ago

Note the output above is exactly what @sebmck was suggesting, so I suppose I can only really say I'm endorsing it!

guybedford commented 9 years ago

@eventualbuddha that resolver looks very interesting - I've been trying to get Traceur to do something similar.

caridy commented 9 years ago

@johnjbarton I'm sympathetic with your ideas, but we should consider the two sides of this coin, a library/package is not a application. Let me elaborate more:

A library (or package by its npm form) is a piece of code that you will require from another library or from an app. the question here is: how can I use a library written in ES6 from a CommonJS module? As today, we have 2 options:

  1. assume consumers of the library will have a ES6 loader in place (either by defining a loader or hijacking require).
  2. transpile the library to CJS to facilitate interoperability without having to introduce extra layers (this also means you have control over the api, e.g.: removing the need to use .default since you can provide an custom API for the CJS interface).

The problem is that these two options looks very different if you are looking from the perspective of a library developer vs application developer. For an application developer, you will probably use option 1) without too much hazard, maybe bringing es6-module-loader or es6-micro-loader, or any other form of loader to support importing es6 and cjs dependencies at the app level. But from the perspective of a library developer, choosing option 1) is completely unacceptable, because using your library as a dependency means more complexity, and those complexity will transcend to the consumer library as well.

In our case, we are mixing both in all of our libraries, e.g.: https://github.com/yahoo/intl-messageformat, creating a shim for the CJS api of our libraries written in ES6 format, and using esnext:main directive to point to the entry point for the ES6 implementation in case consumers are writing code in ES6 as well.

As for the other subject, importing CJS from a ES6 module, I think we should keep it simple:

import fs from "fs"
export {default as fs} from "fs";
export var readFile = fs.readFile;
johnjbarton commented 9 years ago

When node supports es6 modules, will library developers have incentive to publish es6 modules or only cjs modules? That is the important question from my point of view.

Here is my understanding:

If app developers can import or require() a module and succeed, independent of the published format, then it won't matter to lib developers. Great, they can choose es6 format!

But during the interop phase, app developers can require() a module and always succeed, but if they import {name} from 'module' then they will fail if the lib dev published in cjs. This creates an incentive for app devs to always require() and lib devs to always publish in cjs.

Does this dynamic change when node supports es6 modules? I guess no, and that is my concern.

Of course cultural pressure can overcome practical concerns and app devs may choose import over require(). But I see the opposite pressure outside of this group of es6 advocates.

Am I mistaken here?

BTW I don't think arguments about the complexity of the interop implementation make sense. The delta around __esModule / isModule is minuscule compared to the effort behind any of these es6- projects or against a long tail of backwards compat work.

And wow we've come a long way it's great to have these problems ;-)

jjb

caridy commented 9 years ago

But during the interop phase, app developers can require() a module and always succeed, but if they import {name} from 'module' then they will fail if the lib dev published in cjs. This creates an incentive for app devs to always require() and lib devs to always publish in cjs.

this is not true. If you decide to import a module in your app is because you have a loader that can potentially understand when a module is cjs or es6, that's what we do with es6-micro-loader for example, you will be able to do System.import('fs') or System.import('./path/to/mod.js'), and it just work, just like the equivalent declarative syntax, but that's at the app level, where everything is more flexible, the problem is creating reusable piece of code (packages).

jamiebuilds commented 9 years ago

Could this be solved for node users just by using something like 6to5's require('6to5/register');? If we told module authors to simply make the "main" file in their package.json a sort of register file that could expose the package as either cjs or es6, then it would be much simpler for them.

// user-project/node_modules/foo-module/index.js
require('whatever-transpiler/export-package')(module, './lib/foo-module');
// user-project/node_modules/foo-module/lib/foo-module.js
export var foo = 'foo';
// user-project/cjs.js
var foo = require('foo-module').foo;
// 'foo'
// user-project/es6.js
import {foo} from 'foo-module';
// 'foo'
azproduction commented 9 years ago

Could this be solved for node users just by using something like 6to5's require('6to5/register');?

It solves this issue only for node.

I am planning to use 6to5 in a front-end project (AMD). require('6to5/register') is not an option for me that is why I like @guybedford solution (__esModule + isModule).

jamiebuilds commented 9 years ago

It solves this issue only for node.

Browser users will probably need a different solution than node users. People are used to compiling for the browser, but it'll be harder to convince people in node to compile their modules all the time.

sebmck commented 9 years ago

As of 1.12.21 there's a module formatter in 6to5 called commonInterop that'll essentially transpile down import foo from "foo"; to var _ref = require("foo"); var foo = foo && (foo["default"] || foo);.

azproduction commented 9 years ago

Thanks, @sebmck!

guybedford commented 9 years ago

@sebmck this is an amazing move in the right direction!

To confirm - the use case here is that I use commonInterop when I want to write an application consisting of ES6 and CommonJS, and compile the ES6 individually into CommonJS? Would you encourage users to publish these modules to npm?

sebmck commented 9 years ago

@guybedford Yeah that is the use case. commonInterop is currently the default when using bin/6to5-node. I'm considering making it the default but I'll have to put more thought into it. Re: encouraging users to publish these modules, there isn't really anything discouraging it and I'm unsure of how to encourage it?

guybedford commented 9 years ago

Yes that is the point - we can't stop users from doing these things so need to work out interop to the depths of these scenarios, and with ES6 adoption. One of my worries is an interop format itself becoming a hindrance to module unification by being an incompatible format with the module loader. For example, modules that mix CommonJS and ES6 syntax.

The result of this format works, and leads to the following important implications for users:

  1. Dual-publishing workflows should be avoided entirely. That is we should never ever move to a world where we publish ES6 and CommonJS together on npm and have ES6-aware loaders substitute CommonJS for ES6. Clarification - I mean a single package containing both the ES6 and CommonJS files.
  2. Module loaders will need to implement Module objects for CommonJS that contain a default as well as all the exported properties copied down to the base module object to allow these CommonJS named exports.

I actually can get on board with both of these directions, so think I might actually about the situation be happy for now! If I think of anything else will post back here.

I think (1) was clouding the discussion mostly. Then I can make sure to implement (2) so we don't have grumpy users next year complaining this worked in 6to5 but not in actual ES6 loaders.

Note also, there is still some confusion to users whether to use import * as fs from 'fs' or import fs from 'fs', but I suppose this inconsistency doesn't lead to anything too critical.

guybedford commented 9 years ago

@sebmck perhaps we should consider that if there is only one default export in interop mode, set that to be the module.exports value? The default picking up feature will make this work out nicely with the interop.

sebmck commented 9 years ago

@guybedford That's definently a logical next step.

sebmck commented 9 years ago

@guybedford Added as of 1.13.11

guybedford commented 9 years ago

@sebmck that default-collapsing sounds great. My only remaining worry here is that we get further support of allowing named exports for CommonJS. Currently there still seems to be mixed opinion. The worry is that we are giving users a feature, and having done that it is very difficult to remove that feature later as it has set expectations. Basically it's whether or not ES6 environments will create a module object like https://gist.github.com/guybedford/a1155602a0ff41b22359. If we don't have full buy-in from all future ES6 environments then we will have interop pain, so I would strongly advise more evangelism.

guybedford commented 9 years ago

To follow up on dual-publishing of CommonJS and ES6 it actually looks like it will still be possible with this transpilation method. That is, we could publish two folders - the original ES6 alongside the the CommonJS transpiled from this interop mode, and then substitute the CommonJS folder for ES6 folder in ES6 environments with everything still working correctly.

This will be critical for the npm transition path, which will be interesting to hear as it develops further.

The __esModule stuff is a way to avoid the interop issues caused by users using import * to import CommonJS modules, and also avoid issues where existing CommonJS modules have exports.default. Both of these lead to transpiled code that works, but ES6 code that breaks in ES6 environments. It could still be useful to have this flag for these cases, but also with some luck these scenarios should hopefully be edge-case enough not to matter than just being the odd bug for the odd user, and seem relatively self-correcting anyway when they do occur. If the issues get bigger, I guess we can always add it later.

It seems we're actually at a stable output for now, so I'd be happy to close this issue. If anyone feels certain interop perspectives have been overlooked please do post further though.

johnjbarton commented 9 years ago

On Tue, Nov 25, 2014 at 5:10 AM, Guy Bedford notifications@github.com wrote:

To follow up on dual-publishing of CommonJS and ES6 it actually looks like it will still be possible with this transpilation method. That is, we could publish two folders - the original ES6 alongside the the CommonJS transpiled from this interop mode, and then substitute the CommonJS folder for ES6 folder in ES6 environments with everything still working correctly.

I'm sorry but I don't follow this paragraph. What I read is "this transpilation method" leads to "dual-publishing" and "dual-publishing" means we only need the CommonJS publishing path.

Maybe you mean that this transpiration format allows ES6 to be consumed as CommonJS today and as ES6 in future? But we already know that, so I'm still missing something important.

To me the critical requirement in interop is for ES6 import of modules written in ES6 have no constraints if transpiled to commonjs format. That is, every syntax for import and export would be supported. Is that now possible?

Thanks, jjb

guybedford commented 9 years ago

@johnjbarton yes you're interpreting it exactly right - the question was about ensuring this format can be replaced gracefully with the original ES6 and for code to still work correctly. It does still require testing for some of the edge cases:

But it seems that it will work in the majority and those cases will be covered pretty quickly by basic testing.