google / traceur-compiler

Traceur is a JavaScript.next-to-JavaScript-of-today compiler
Apache License 2.0
8.17k stars 580 forks source link

traceur.get alternatives #2052

Closed guybedford closed 8 years ago

guybedford commented 8 years ago

This function has been critical to allow creating custom transformers for use with Traceur given the current API.

For an example see how we compile CommonJS modules in SystemJS builder at https://github.com/systemjs/builder/blob/master/compilers/cjs.js.

The current workaround involves replacing this use of traceur.get with eg:

$traceurRuntime.ModuleStore.get('traceur@0.0.95/src/codegeneration/ParseTreeTransformer.js');

Where $traceurRuntime is the environment global.

I tried adding everything needed to compiler-imports.js but it quickly becomes obvious that this isn't an approach that will scale, although can provide the PR to cover my own use cases if that is preferred over the traceur.get API?

Otherwise perhaps a comprehensive export * mechanism is needed here to provide all the namespaced APIs for transformer creation?

johnjbarton commented 8 years ago

Far and away the best solution to accessing eg ParseTreeTransformer is

import {ParseTreeTransformer} from './ParseTreeTransformer.js';

and transcoding especially in the cjs example. Maybe we can do something to make it easier to take this path?

Next best would be expanding the API via more export calls. But as you note our API is (deliberately) incomplete. Perhaps we should reduce our use of the API now that we have es6 based testing, to further encourage transpiling modules over es5 API calls? That said, I'm not opposed to expansion if the alternative is very painful.

I wouldn't even put $traceurRuntime.ModuleStore.get on the list, I thought it was internal API only global because we have to do that for bootstrap.

guybedford commented 8 years ago

It's really about if Traceur wants to encourage users to create their own transformers with it. I've already built my transpilers with Traceur so that's the reason for me, but I also feel that Traceur is potentially a faster compiler than Babel for these cases (although haven't done the deeper performance analysis).

I do think extending the API sounds like the best way here. Let me look again at doing a PR for my needs. In terms of scaling the approach, I think just being able to structure it into subfiles to allow export * would help manage this file better. I can possibly attempt something along these lines as well.

johnjbarton commented 8 years ago

So you don't think users will use es6 to create transformers?

Subfiles that matched the namespace hierarchy, eg codegeneration for traceur.codegeneration would make sense to me.

arv commented 8 years ago

We used to map everything into es5 style namespace objects. We went away from that because es modules provided a cleaner abstraction.

What we should do is allow import {ParseTreeTransformer} from 'traceur/src/codegeneration/ParseTreeTransformer.js' in an npm environment.

guybedford commented 8 years ago

If allowing the above, then perhaps it would make sense to do a separate file dist build of Traceur as well?

Along the lines of -

traceur --dir src dist --modules=commonjs

The issue otherwise is the graph duplication of having to load effectively two versions of Traceur - one for the transformer and one for the main global itself.

arv commented 8 years ago

Yes! We should do that!

I would probably compile it to dist/commonjs and also make sure we include src/ in the npm output.

arv commented 8 years ago

Snags. We need to add a way to not include all files in a directory since we have those *.js-template.js files.

arv commented 8 years ago

With those 2 changes and ./traceur --dir src/ dist/ --modules=commonjs the following works as expected:

'use strict';

require('./bin/traceur-runtime.js');

const ParseTreeTransformer = require('./dist/codegeneration/ParseTreeTransformer.js').ParseTreeTransformer;
const Script = require('./dist/syntax/trees/ParseTrees.js').Script;
const parseStatements = require('./dist/codegeneration/PlaceholderParser.js').parseStatements;
const parseExpression = require('./dist/codegeneration/PlaceholderParser.js').parseExpression;
const STRING = require('./dist/syntax/TokenType.js').STRING;
const LiteralExpression = require('./dist/syntax/trees/ParseTrees.js').LiteralExpression;
const LiteralToken = require('./dist/syntax/LiteralToken.js').LiteralToken;
const IdentifierExpression = require('./dist/syntax/trees/ParseTrees.js').IdentifierExpression;
const IdentifierToken = require('./dist/syntax/IdentifierToken.js').IdentifierToken;
const BindingIdentifier = require('./dist/syntax/trees/ParseTrees.js').BindingIdentifier;
const createUseStrictDirective = require('./dist/codegeneration/ParseTreeFactory.js').createUseStrictDirective;

console.log(
    ParseTreeTransformer,
    Script,
    parseStatements,
    parseExpression,
    STRING,
    LiteralExpression,
    LiteralToken,
    IdentifierExpression,
    IdentifierToken,
    BindingIdentifier,
    createUseStrictDirective);
guybedford commented 8 years ago

@arv how does the performance compare for startup time?

arv commented 8 years ago

@guybedford. Good question. I haven't checked. I'll run some tests when I get the time.

arv commented 8 years ago

@guybedford Using the code snippet in the comment above. test.js is using dist and and test2.js is using traceurGet from https://github.com/systemjs/builder/blob/master/compilers/cjs.js:

$ time node test.js
120ms

real    0m0.175s
user    0m0.153s
sys 0m0.027s

$ time node test2.js
256ms

real    0m0.312s
user    0m0.278s
sys 0m0.042s

In other words, performance is better in your case.

Changing this to import src/traceur.js which pulls in almost everything I get:

$ time node test.js
237ms

real    0m0.295s
user    0m0.271s
sys 0m0.036s

$ time node test2.js
249ms

real    0m0.311s
user    0m0.278s
sys 0m0.042s

So the performance is still in favor for the dist version.