Transpiler handling

[guybedford]

I'm wondering where to go with the transpiler handling for this project.

The internal transpilation layer is great for seeing a demo how things work, and is completely necessary to replicate the full pipeline, but I'm wondering if we really need it with pluggable transpilers, or if we shouldn't just create for example a custom build of Traceur or Babel that only does module conversion into System.register and embedding that within the build as the only option.

[johanneslumpe]

@guybedford If there is only a custom build which does the module conversion, how would we then transpile other features? Have a separate transpile step from raw es6/7 to es5 which leaves the import and export statements intact so that they can be transformed by the loader?

[guybedford]

@johanneslumpe SystemJS has actually already solved this problem by treating all dependencies as dynamic modules in the pipeline, and reimplementing the zebra striping linking algorithm for CJS / ESM circular references internally. That is, SystemJS only uses the "dynamic" custom module part of this pipeline, while replicating the rest itself. It's as if the module loader can't load ES modules at all, and can only load through the legacy layer, which is the nice polyfill path. Hence the removal of focus on transpilation in this project.

[johanneslumpe]

@guybedford Oh that's great! My use-case is basically that I want to load transpiled es6 modules transparently without having to worry about fetching the default export when using require. I don't want to add a transpiler to the loader as everything is already transpiled, so this change would help with that I assume.

[jolyndenning]

Hmm I'm hesitating a bit about this suggestion for a couple of reasons. If I understand correctly, this would mean we don't fully implement the spec here in es6-module-loader, but instead lean on SystemJS to perform much of the actual work. Specifically, if es6-module-loader is unable to transpile import and export statements, it would not be able to implement any of the Linking stage of modules.

I think some of the reason why I'm hesitating on this is because es6-module-loader would really not be useful without a hard dependency on SystemJS. Nor would it really be a polyfill of the whatwg/loader spec. Additionally, SystemJS may never be able to move off of the es6-module-loader polyfill if we go with something that is not in the spec.

I'm not an expert in zebra striping nor in the technical differences between implementing dynamic modules and static modules, so maybe what I'm about to say doesn't make a ton of sense. But what I'm thinking is that SystemJS would take care of (1) transpiling all the non-module-loading stuff via the translate hook and (2) converting CJS syntax to ESM syntax during the same translate hook. Then es6-module-loader would take care of computing dependency graphs, loading dependencies, evaling the module with a correct closure that includes the dependencies, etc. In other words, es6-module-loader doesn't do any transpilation except what is required to understand import and export statements.

Does that make sense? I know that it's a pretty dramatic shift from where es6-module-loader and SystemJS are today. Especially (2).

[guybedford]

@joeldenning to clarify the specific point I'm making is to deprecate the transpiler option that adds other ES feature compilations, and to just support one builtin parser / transpiler that only does the module syntax handling. This way we can make that a custom Babel build that is included with this project as the only way to use the linking system.

[jolyndenning]

Ah I see. In that case, yes I completely agree that we should do that.

[backspaces]

This is exactly what I've tried to build using transpiler configs like below .. which didn't work, apparently broke es6-module-loader. I'd LOVE a modules-only transpilation!

System.traceurOptions = { arrowFunctions: false, blockBinding: false, classes: false, computedPropertyNames: false, defaultParameters: false, destructuring: false, forOf: false, generators: false, numericLiterals: false, propertyMethods: false, propertyNameShorthand: false, restParameters: false, spread: false, symbols: false, templateLiterals: false, unicodeEscapeSequences: false, unicodeExpressions: false }

[probins]

just support one builtin parser / transpiler that only does the module syntax handling

How big would such a parser be? Current/upcoming versions of main browsers now support more ES6 than traceur/Babel, so transpiling no longer makes much sense IMO. The problem however has always been that to handle import/export you need a huge parser.

[guybedford]

It would be the size of Babel without any plugins.

[guybedford]

Note though that this is still not a production-suitable workflow, so transpiler size is not a big concern.

[jolyndenning]

Since my first couple of comments on this thread, I've been working on a babel-less compiler that compiles the imports and exports of es6 modules into System.register syntax. Right now it is very much just a working prototype, but my hope is that it could become a viable solution for es6-module-loader production workflows, since it has no dependencies and is 5K gzipped. The implementation has progressed such that it fully works for about 15 es6 modules, which it has tests for.

I'm interested in your feedback on it: the implementation method I've chosen is fast and has a small footprint, but that comes with some doubt in my mind about whether I'd really be able to pull off complete correctness for all es6 modules. Do you think it's worth pursuing?

https://github.com/joeldenning/kremling

[probins]

Note though that this is still not a production-suitable workflow, so transpiler size is not a big concern

The other way round surely: not prod because of size :-)

Iirc what Joel is attempting is how the polyfill used to work, before using a separate transpiler

[jolyndenning]

Yeah looks like es6-module-loader used to use esprima.

[probins]

There's another issue with not transpiling ATM, that uglify doesn't yet support es6. Last time I looked, this was being worked on in a separate branch.

[guybedford]

@joeldenning this is a really great idea and certainly something that would help this project for the transition to ES modules. Having a transpiler just for modules that is both small and performant is definitely something we want.

But correctness becomes critically important. And I think that is only possible with a full JS tokenizer, which I can't see implemented there? The rules of JS language tokenizing are not straightforward unfortunately. For example there are cases like the inability to distinguish if a / is a regular expression or division sign, which then completely alters the tokenizer state as parsing in a regex is a different mode to normal js. Note for ES6 backtick comment expressions also need to be implemented. In the end it may be best to just build on top of an existing tokenizer to keep the overhead down, although I'm not even sure if JS is even possible to parse without some level of ast due to the various intricate parsing lookahead rules that need to be catered for.

[probins]

I think this is becoming increasingly necessary. Babel no longer supports running in the browser and no longer produces (https://babeljs.io/docs/usage/browser/) the browser.js file mentioned in the readme, and there seem to be limitations to running Traceur (see #477). Not sure it's worth spending a lot of time on though. Depends on how long it will be before we have a usable implementation of import/export in a browser; even if it's behind a flag and not usable for production, it could be used for development.

[probins]

uglify doesn't yet support es6. Last time I looked, this was being worked on in a separate branch.

this comment doesn't really belong here, as the loader doesn't use Uglify, but I'd suggest changing to the harmony branch of Uglify throughout all the jspm and systemjs workflows. AIUI, this doesn't yet support all of ES6, for example, generators, but should enable removing transpile except for import/export for those who want to use native minimised ES6.

[jolyndenning]

I've been thinking over this problem a lot recently, and realized that @guybedford is right that it would probably be really difficult for the kremling compiler I wrote to completely accurately compile es6 down to System.register. However, I came up with another solution that I believe would completely work all the time. There's one major problem that I know of with it, but thought I'd put it here and maybe you guys would either find a way to solve that problem or maybe find other problems with it.

My solution is slightly different than the normal System.register format, but could work either with a vanilla System.register implementation or a slightly adjusted System.register implementation. The idea is that the compiler complexity is really caused by having to call $__export whenever an exported value changes. My approach would make it so that modules _do not need to call $_export when their exported values change. The rest of the compiler (converting import and export into System.register) is very straightforward and I have a lot of confidence that kremling or a similar compiler could handle that.

The way that you can get away with not calling $__export when exported variables change is by defining getters and setters on the global/window object. So a module that vends exports would actually have all of its exports be variables on the window, instead of local to the function declared inside of the System.register call. During the initialization of the module, getters and setters for each exported variable would be set up. When the variable is accessed or written, the getter and setter would determine who was the one accessing/writing the variable, and provide a different value based on who was accessing it. That way, modules can export variables of the same name, and also you can default to the actual value on the window when the person accessing the variable is not one of the modules that exports or depends on a variable of that name. The way that you determine who is accessing the global variable is by looking at caller.name inside of the getter/setter. The calling function's name (or maybe caller.caller.name or caller.caller.caller.name) eventually would be the name of the function declared as the System.register function.

Code example (just shows the basics of what I mean, is not by any means complete):

System.register('foo.js', [], function __System__register__foo() {
  return {
    setters: [],
    execute: function() {
      setupExportGettersSetters('value1', 2);
      setupExportGettersSetters('value2', function() { return value1 });
      value1 = 3;
   }
})

System.register('bar.js', ['foo.js'], function __System__register__bar() {
  setupGetter('value1', 'foo.js');
  setupGetter('value2', 'foo.js');
  return {
    setters: [],
    execute: function() {
      console.log(value1);
      console.log(value2);
   }
})

function setupExportGettersSetters(name, value) {
  Object.defineProperty(window, name, {
    get: function() {
      var whoIsGettingTheValue = searchCallerName(caller);
      if (whoIsGettingTheValue) {
        getValue(whoIsGettingTheValue);
      } else {
        return actualGlobalValue;
      }
   },
   set: function() { ... }
}

function searchCallerName(theCaller) { ... }
function setupGetter(propName, depName) { ... }
function getValue(who) { ... }

Anyway, if that explanation and code sample are intelligible enough to get the idea of what I'm describing, I think that it would completely work (even though that code sample is far from complete). The one big problem with it, though, is that caller is not available in strict mode, which means that we would have to find a different way of determining who is getting/setting an exported value.

[kevinSuttle]

I'm not understanding. The docs say:

It is important that Babel, Traceur or TypeScript is installed into the path in order to be found, since these are no longer project dependencies.

I have Babel installed, but I'm getting "critical errors" from this module by way of Traceur.

CLI:

WARNING in ./~/es6-module-loader/index.js
Module not found: Error: Can't resolve 'babel/browser.js' in '/Users/kevinSuttle/Code/platform-ui-primitives/node_modules/es6-module-loader'
 @ ./~/es6-module-loader/index.js 17:58-93

WARNING in ./~/es6-module-loader/index.js
Module not found: Error: Can't resolve 'typescript/bin/typescript.js' in '/Users/kevinSuttle/Code/platform-ui-primitives/node_modules/es6-module-loader'
 @ ./~/es6-module-loader/index.js 21:41-88

WARNING in ./~/es6-module-loader/index.js
Module not found: Error: Can't resolve 'babel-core/browser.js' in '/Users/kevinSuttle/Code/platform-ui-primitives/node_modules/es6-module-loader'
 @ ./~/es6-module-loader/index.js 13:36-76

WARNING in ./~/traceur/bin/traceur.js
Module not found: Error: Can't resolve 'vertx' in '/Users/kevinSuttle/Code/platform-ui-primitives/node_modules/traceur/bin'
 @ ./~/traceur/bin/traceur.js 2578:18-28

WARNING in ./~/traceur/bin/traceur.js
1023:38 Critical dependency: the request of a dependency is an expression

WARNING in ./~/traceur/bin/traceur.js
1023:102 Critical dependency: the request of a dependency is an expression

WARNING in ./~/traceur/bin/traceur.js
25877:45 Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

WARNING in ./~/traceur/bin/traceur.js
26080:45 Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

WARNING in ./~/traceur/bin/traceur.js
26128:45 Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

WARNING in ./~/traceur/bin/traceur.js
26152:45 Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

WARNING in ./~/traceur/bin/traceur.js
26205:45 Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

WARNING in ./~/traceur/bin/traceur.js
26234:45 Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

WARNING in ./~/traceur/bin/traceur.js
26277:45 Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

WARNING in ./~/traceur/bin/traceur.js
26525:45 Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

WARNING in ./~/traceur/bin/traceur.js
26810:45 Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

ERROR in ./~/es6-module-loader/dist/es6-module-loader-dev.src.js
Module not found: Error: Can't resolve 'fs' in '/Users/kevinSuttle/Code/platform-ui-primitives/node_modules/es6-module-loader/dist'
 @ ./~/es6-module-loader/dist/es6-module-loader-dev.src.js 1529:17-30

ERROR in ./~/traceur/bin/traceur.js
Module not found: Error: Can't resolve 'fs' in '/Users/kevinSuttle/Code/platform-ui-primitives/node_modules/traceur/bin'
 @ ./~/traceur/bin/traceur.js 33500:21-34

ERROR in ./~/traceur/bin/traceur.js
Module not found: Error: Can't resolve '../node/nodeLoader.js' in '/Users/kevinSuttle/Code/platform-ui-primitives/node_modules/traceur/bin'
 @ ./~/traceur/bin/traceur.js 33491:23-55

ERROR in ./~/source-map-support/source-map-support.js
Module not found: Error: Can't resolve 'fs' in '/Users/kevinSuttle/Code/platform-ui-primitives/node_modules/source-map-support'
 @ ./~/source-map-support/source-map-support.js 3:9-22

Browser:

Cannot read property 'match' of undefined
TypeError: Cannot read property 'match' of undefined
    at Object.eval (webpack:///~/es6-module-loader/index.js:6:45)
    at eval (webpack:///~/es6-module-loader/index.js:30:30)
    at Object.<anonymous> (webpack:///./~/es6-module-loader/index.js?b501:2996:2)
    at __webpack_require__ (http://localhost:9001/static/preview.bundle.js:556:30)
    at fn (http://localhost:9001/static/preview.bundle.js:87:20)
    at eval (http://localhost:9001/static/preview.bundle.js:925:2)
    at Object.<anonymous> (http://localhost:9001/static/preview.bundle.js:925:2)
    at __webpack_require__ (http://localhost:9001/static/preview.bundle.js:556:30)
    at fn (http://localhost:9001/static/preview.bundle.js:87:20)
    at eval (http://localhost:9001/static/preview.bundle.js:811:2)