Allow ES6 transpiled into CommonJS to load non ES6 dependencies

[guybedford]

Just as with #785, for interop with CommonJS we need to alter the CommonJS output for an import statement:

import id from 'some-module';

from this output:

{ default: id } = require('some-module');

to:

var __module = require('some-module');
if (!__module || !__module.__esModule)
  __module = { default: __module };
{ default: id } = __module

Since transformImportDeclaration currently only transforms a single statement, I wasn't sure how to implement this currently.

[guybedford]

Note that getting this in is a prerequisite to enabling Browserify ES6 modules as far as I can interpret!

[guybedford]

Bumping this after reading http://esdiscuss.org/topic/quantifying-default-exports.

This is a breaking change unfortunately... but may be more important than I thought.

[guybedford]

I would like to work on getting this in, but it will require some buy-in from users since it is breaking.

@briandipalma does this breaking change make sense to you?

[johnjbarton]

I think it would be helpful to explain what will not work after the change. I guess most projects do not save compiled outputs.

[guybedford]

@briandipalma said he was writing something like the following when importing CommonJS from ES6:

module minimist from 'minimist';

This was because the CommonJS compiler turns this into var minimist = require('minimist');

This should really be:

import minimist from 'minimist';

And that's where this change is breaking.

Actually, since the module syntax is changing anyway, this is a breaking change that would happen anyway.

[briandipalma]

Yes I expected that code to have to be rewritten eventually so this isn't an issue for me.
Whatever key you decide to export module.exports as is good for me.

I saved the output as I could publish some of these tools to npm and wouldn't want people to have to compile the code when they install the package. I have a generator for ES6 node projects so they all have the same structure.

[UltCombo]

The ES6 module syntax is still a draft, so I don't see any problems with breaking changes.

As long as your suggested patch is still spec-compliant and supports requiring node core modules, I'll +1 it.

By the way, what about the new import * as foo from 'foo' syntax? Wouldn't it work for node core modules out of the box?

[UltCombo]

Testing against Traceur 0.0.53, this code:

import * as fs from "fs";

Generates:

"use strict";
var fs = require('fs');

I believe this is the proper way to go around this, plus this is the exact example shown on JavaScript Modules.

No need to add hacks for non-standard syntax IMO. import x from 'foo' is expected to assign foo's default export to x, that should throw an error until Node supports ES6's default exports. In the end it will be less confusing to end-users when/if Node adds default exports support, as we won't have to explain that their traceur-compiled code worked until then due to some non-standard magic extension.

[domenic]

@UltCombo That generated code will be incorrect for modules that do module.exports = /* a function */. The ES6 code import * as x from "y" will never produce a x that is a function, but var x = require('y') can produce such an x. This is very bad, as it causes people to write "ES6" code that is not really ES6, just because it works well with Traceur.

This is a hard problem to solve. IMO interop between Node modules and ES6 module syntax should not be attempted. Instead I just use require for Node modules and import for ES6 modules. But I guess the very existence of the CJS transformer means Traceur has slightly different goals in that respect.

[UltCombo]

@domenic Oh I see, that makes perfect sense. I also use require() for node core modules, personally. And I agree that it makes more sense than writing ES6 code that is not really ES6.

[johnjbarton]

@domenic please post a specific example of "ES6" code that causes traceur
to generate incorrect code.

The CJS transformer exists to allow ES6 code to run in today's nodejs
engine. It allows one specific kind of interop: ES6 can run under nodejs.
If there are cases where it fails please let us know.

On Mon, Aug 4, 2014 at 1:06 AM, Fabrício Matté notifications@github.com
wrote:

@domenic https://github.com/domenic Oh I see, that makes perfect sense.
I also use require() for node core modules, personally. And I agree that
it makes more sense than writing ES6 code that is not really ES6.

—
Reply to this email directly or view it on GitHub
#818 (comment)
.

[guybedford]

@johnjbarton a specific example would be exactly as described in #818 (comment).

Traceur is effectively setting a precedent whether it means to or not in terms of interop.

@domenic I understand your interop concerns, but as long as there are compilers which compile ES6 to CommonJS, loading non-ES6 from those modules will always be a scenario. We can either pretend this case doesn't exist even though it does, or we can implement the best possible fix.

In terms of the interop problem, I don't think it is that bad.

The behaviour is pretty much identical to what you would get with the examples on jsmodules.io.

We get import fs from 'fs' out of the box, but not import { readFile } from 'fs', which requires ES6 syntax.

There is one other suggestion for making interop smoother and that is an export flag for CommonJS modules to indicate how they want to be consumed in ES6. Setting exports.__esModule = true; on a CommonJS module would effectively inform an ES6 importer to use the latter case when importing it from ES6 as opposed to the former.

That's the simplest I've been able to come up with. Further suggestions welcome.

[johnjbarton]

Sorry I still don't follow. Is Traceur taking valid ES6 code to valid ES5 code or not?

[guybedford]

@johnjbarton when transpiling modules to CommonJS, there is no question of spec validity, the question is simply how modules interop, which is something that has been discussed quite a bit, but is also very tricky to discuss without sitting down and thinking about the push and pull between the two systems.

To try to clarify the example:

import * as fs from 'fs';

Will currently compile as a single module into (when not doing a tree build):

var fs = require('fs');

When running this module in Node, we are now loading a normal NodeJS module from transpiled ES6. The fs variable is exactly the fs variable from Node.

But in the dynamic loader, ES6 can load from CommonJS with that same syntax:

import * as fs from 'fs';

In this scenario, the Module object for CommonJS would look like - Module({ default: fs }). The reason for this is that we don't know in advance if a CommonJS module is a plain object, or a complex object, so we act as if it has the potential to be more complex.

So we would really expect to import the module with:

import fs from 'fs';

Thus for the two interop scenarios to line up, I am suggesting changing the output of Traceur from the first example, to match the second example rather.

[UltCombo]

@guybedford So, basically, you are proposing to interop ES6 import syntax with CJS modules by transforming the native CJS modules (node core modules) from require('foo') into (what would be in CJS syntax) require('foo').default (CJS module foo becomes { default: foo }), so that import foo from 'foo' works as expected?

I agree this is a better solution than import * as foo from 'foo', as CJS can export virtually any kind of value (function, scalar values), while import * as foo from 'foo' transforms into var foo = require('foo') which would possibly allow invalid ES6 code to work (e.g. import * as foo from 'foo'; typeof foo === 'function') as @domenic mentioned.

[guybedford]

@UltCombo that is the right idea, but rather than needing to transform native CJS modules, we can handle this with detection in the module output itself that is importing from a CommonJS module:

app.js

import fs from 'fs';

->

var __fs = require('fs');
if (!__fs || !__fs.__esModule)
  __fs = { default: __fs };
var fs = __fs['default'];

We detect if the module is ES6 in the compiled output, forking our behaviour at that level, so that this interop is independent of the environment. We do this with the __esModule flag on compiled ES6 into CommonJS to know the difference.

[UltCombo]

@guybedford

that is the right idea, but rather than needing to transform native CJS modules, we can handle this with detection in the module output itself that is importing from a CommonJS module

Yes, I understood that bit -- I just preferred to write in a different manner as @johnjbarton seemed to have trouble understanding your suggestion. =]

I'm wondering if we can get a cleaner way to detect CJS modules.

[johnjbarton]

Transcoding

import fs from 'fs';

to

System.syncImport('fs');

would also avoid these issues.

[UltCombo]

@johnjbarton pardon me, but I can't seem to find any synchronous version of System.import in the ES6 spec draft nor in the es6-module-loader polyfill.

[johnjbarton]

My bad. I meant: rather than transcoding to a pre-existing function
designed for es6 node modules, transcode to a function designed for es6
node modules.

But maybe this too is wrong. Maybe the whole concept behind our CJS mode is
wrong for es6. Or es6 is wrong in not have a sync version of System.import.

On Mon, Aug 4, 2014 at 4:28 PM, Fabrício Matté notifications@github.com
wrote:

@johnjbarton https://github.com/johnjbarton pardon me, but I can't seem
to find any synchronous version of System.import in the ES6 spec draft
nor in the es6-modules-loader polyfill.

—
Reply to this email directly or view it on GitHub
#818 (comment)
.

[UltCombo]

@johnjbarton yes, AFAIK, since the beginning of the ES6 module syntax spec it was clear that it wouldn't be 100% compatible with CJS, and a lot of Node.js people argued over this already.

Seeing as Node currently only supports CJS, it makes sense to convert the ES6 modules into CJS for better Node compatibility (module resolution, caching, etc.) -- not like this logic couldn't be re-implemented, but reinventing the wheel would most likely be a waste of time and bring many more corner case bugs. At least this is how I see it.

The ES6 -> CJS transformation seems fine to me, the problem is CJS -> ES6.

Since Node uses CJS, I don't really mind having require() calls in my ES6 Node code. I'm not sure if it is worth adding hacks to support syntactic-sugaring this.

[johnjbarton]

But the module resolution logic has to be in the normalize/locate/fetch functions. Any difference between the ES6 Loader and require() is just a bug. The Loader is already a cache. So any gain in implementation via CJS is an illusion.

The logic of ES Module loading is to load a root module with a platform-dependent async call, then synchronously load and compile any dependents. There is no particular reason to implement this by creating CJS modules and loading them with require(). It just leads to all kinds of arguments and issues about how the resulting CJS modules are "not real node modules". What if we created .esm files and generate calls to loadESM() to load them? These files would contain the modules=instantiate output broken out into files. Then interop with node is simple: call require().

If we back up even further, maybe we should focus on a node version of System that works well. That is how we begin a program of cross-platform ES6 modules. Otherwise we are always second rate node modules. Once our System story is solid and we have cross-platform working, then we can optimize with pre-compilation.

[UltCombo]

@johnjbarton

But the module resolution logic has to be in the normalize/locate/fetch functions. Any difference between the ES6 Loader and require() is just a bug. The Loader is already a cache. So any gain in implementation via CJS is an illusion.

Oh right. I still see a couple use cases where a centralized module loading system is simpler in practice, e.g. overriding/hacking it for unit testing purposes (example).

It just leads to all kinds of arguments and issues about how the resulting CJS modules are "not real node modules". [...] Otherwise we are always second rate node modules.

Sorry, not quite sure what you mean there. You mean converting esm to cjs is a degradation, while using the System API is perfectly fine?

From a logical standpoint, I agree that using the System API makes more sense for ES6 code, but in the end what we are actually running is transpiled ES5 code.

If we back up even further, maybe we should focus on a node version of System that works well.

I'm not quite sure where you're trying to get at. What problems do CJS-compiled Traceur modules have that would be solved using System? And would it be worth the effort of implementing Node's module resolution logic and possibly a synchronous import version? I'm not sure if synchronicity would be required provided the System API is used correctly. Would there be more complications?

[johnjbarton]

You mean converting esm to cjs is a degradation, while using the System API is perfectly fine?

No. I mean that require() is defined for cjs and System is defined for ES6 modules. Encouraging node users to use require() for ES6 modules splits the ES ecosystem. Encouraging node users to call System for ES6 modules eases the transition to ES6.

I'm not sure if synchronicity would be required provided the System API is used correctly.

I agree. We only need sync calls for the possible optimization of ES6->ES5. When we precompile ES6 to ES5 for node under cjs we create one file for every module even though these modules are really nodes in the compilation dependency graph. Then we re-assemble the dependency graph by reading the files. In creating the decomposed graph on disk we chose a format that requires synchronous reads to reconstitute.

Reading these modules back in a file at a time, synchronously, is inefficient. But it does allow use to share the compilation time across multiple uses of these modules. I believe for most projects this is the wrong trade-off. Most projects would be better off with a single monolithic file that loads fast. (In fact on modern machines it is possible that reading the individual modules synchronously is slower than compiling them. That's why Jikes Java compiler was not driven by make but simply recompiled everything.).

I want to be clear that I am not against efforts to improve the cjs compilation mode. I am against any changes to the ES6 module design solely to allow require() to work with ES6. I am for a good implementation of System for node that demonstrates the ES6 way to work. In the long run (next year) we are not going to precompile for node, we'll just use System and import declarations.

[UltCombo]

@johnjbarton that makes a lot of sense, thanks.

@guybedford I'm not sure about this proposal. ESM syntax interop with CJS means proprietary syntactic sugaring AFAICS. I personally prefer to stay with require() for CJS modules, as it does not involve any proprietary magic.

I'm not opposing your suggestion nor extensions to allow such sugarings though.