esnext / es6-module-transpiler

Tomorrow’s JavaScript module syntax today
http://esnext.github.io/es6-module-transpiler/
Other
1.21k stars 85 forks source link

"illegal yield expression" on nested yields #146

Closed jlongster closed 10 years ago

jlongster commented 10 years ago

I was trying broccoli-es6-module-transpiler on my project and it won't compile my files because it throws an error on nested yields:

function* foo() {
  yield bar(yield 5);
}
% broccoli build build
File: db.js
Error: Line 2: Illegal yield expression
    at throwError (/Users/james/projects/jlongster3/node_modules/broccoli-es6-module-trans
piler/node_modules/es6-module-transpiler/dist/es6-module-transpiler.js:2584:21)          
    at throwErrorTolerant (/Users/james/projects/jlongster3/node_modules/broccoli-es6-modu
le-transpiler/node_modules/es6-module-transpiler/dist/es6-module-transpiler.js:2596:24)  
    at parseYieldExpression (/Users/james/projects/jlongster3/node_modules/broccoli-es6-mo
dule-transpiler/node_modules/es6-module-transpiler/dist/es6-module-transpiler.js:4849:13)
caridy commented 10 years ago

The module transpiler will only attempt to understand module syntax, any other feature should be transpiled ahead of time. You should be able to use broccoli to pick up the AST and pass it around multiple transpilers. /cc @ericf

ericf commented 10 years ago

Supplying AST is not currently supported. Since Broccoli uses the file system as the API between plugins, the AST would need to be serialized, and an AST Resolver class would need to be created, plus the broccoli-es6-module-transpiler would need to be updated to accept other resolvers.

@jlongster My suggestion would be to first use broccoli-esnext, then broccoli-es6-module-transpiler (from master).

mmun commented 10 years ago

Being able to apply AST transformations is very useful. It can be done today by extending Module and the resolver, but I would prefer an API like:

var paths = ['lib/'];
var options = { moduleClass: SweetJSModule };

var container = new Container({
  resolvers: [ new FileResolver(paths, options) ],
  formatter: new BundleFormatter()
});
jlongster commented 10 years ago

The module transpiler will only attempt to understand module syntax, any other feature should be transpiled ahead of time. You should be able to use broccoli to pick up the AST and pass it around multiple transpilers. /cc @ericf

Wat? You should not require standard ES6 features to be transpiled! A non-standard language extension, sure, but not something like generators which are extremely well-defined at this point and handled in latest esprima.

I will compile generators out for production code, but node is getting them soon natively (can have them now if willing to use 0.12) and I fully intend to locally develop against it and not transpile.

ericf commented 10 years ago

@jlongster Looking closer at the stack trace you posted, it appears that you're using an older version of the transpiler and broccoli plugin. Can you try again, but with the latest version to see if throws when your yield statement is encountered?

ericf commented 10 years ago

@jlongster just tried with the latest version and it's compiling things correctly:

Bundle Format:

(function() {
    "use strict";
    function* module$yield$test$$foo() {
        yield module$yield$test$$bar(yield 5);
    }

    function module$yield$test$$bar() {

    }
}).call(this);

//# sourceMappingURL=module-yield-test-compiled.js.map

CommonJS Format:

"use strict";

Object.seal(Object.defineProperties(exports, {
    foo: {
        get: function() {
            return foo;
        },

        enumerable: true
    }
}));

function* foo() {
    yield bar(yield 5);
}

function bar() {

}

//# sourceMappingURL=module-yield-test-compiled.js.map

I misinterpreted your original question…I thought you were wondering why it wasn't compiling your generator to ES5.

jlongster commented 10 years ago

Ah, thanks @ericf. Makes sense. I just did npm install broccoli-es6-module-transpiler, so I guess the version on npm is old.

ericf commented 10 years ago

@jlongster it actually was just updated yesterday ;)