Improve Compiler API

[arv]

Here is a strawman API.

var c = new Compiler(options = new Options, errorReporter = new ThrowErrorReporter)
var tree : ParseTree = c.parseScript(string or file); // file = {name, contents}
var tree : ParseTree = c.parseModule(string or file); // file = {name, contents}
var transformedTree : ParseTree = c.transform(tree : ParseTree);
c.write(tree : ParseTree) : string
c.writeSourceMap(tree) : Object // JSON

Errors are dealt with by passing in an optional error reporter. The default one should just throw SyntaxError, ReferenceError etc.

Source maps are explicit.

[matthewrobb]

I fully endorse exploring this. Esprima/Mozilla Parser API are great but competition is starting to become sorely needed.

How difficult would it be to write an adapter for sending an esprima ast through a traceur transform? Could those ASTs be converted fairly easily?

[johnjbarton]

Currently a source map is a side-effect of writing an AST to string. So maybe

{source, sourceMap} = c.writeWithSourceMap(tree);

Rather than customizing the ErrorReporter via arguments we could customize by extending Compiler, eg overriding a Compiler.reportError(). Functions like parseModule() would clear the error count, parse, then check the error count and throw rather than return. That gets rid of all the checkForErrors()

The API has a lot more ridiculous sounding functions that also need work.

[arv]

Is there a reason we cannot write the source map in a different pass?

[johnjbarton]

I guess the sourcemap generation is less error prone if it extends on the source generation. And if it extends the source generation then lots of string manipulation is duplicated in the second pass.

[johnjbarton]

Do we want to get rid of the async functions? I added them to be all ES6-y but no one wants async after all.

[UltCombo]

@johnjbarton get rid of async functions in what context? How do them relate to this proposed compiler API?

[johnjbarton]

@UltCombo src/Compiler.js, which supplies much of the node api.

[johnjbarton]

Do we want to get rid of the options to each compiler pass, allowing options to change only upon Compiler construction?

Of course this is meaningless until we pass options by argument down into parser/transformers, but then it would prevent options from being changed between parse and transform. That's probably a good thing.

[UltCombo]

@johnjbarton sorry, but I can't seem to find any async function there. The Node API's compile function is synchronous at least.

Do we want to get rid of the options to each compiler pass, allowing options to change only upon Compiler construction?

IMO it doesn't make much difference; if one needs different compile options s/he can instantiate a new Compiler instance (assuming you're talking about the proposed API redesign from OP).

[johnjbarton]

Please see src/Compiler.parse(). On Aug 12, 2014 9:21 AM, "Fabrício Matté" notifications@github.com wrote:

@johnjbarton https://github.com/johnjbarton sorry, but I can't seem to find any async function there. The Node API's compile function is synchronous at least.

Do we want to get rid of the options to each compiler pass, allowing options to change only upon Compiler construction?

IMO it doesn't make much difference; if one needs different compile options s/he can instantiate a new Compiler instance (assuming you're talking about the proposed API redesign from OP).

— Reply to this email directly or view it on GitHub https://github.com/google/traceur-compiler/issues/1233#issuecomment-51938424 .

[UltCombo]

@johnjbarton Oh I see, my bad. Seems like you're promisefying 2 synchronous methods (stringToTree and treeToTree). I'm not sure what benefits that brings.

[domenic]

Yes, please don't promisify synchronous methods.

[briandipalma]

@domenic

Yes, please don't promisify synchronous methods.

Curious as to why you shouldn't? Is it because the resolve is done in sync? What if you added a setTimeout of 0?

[UltCombo]

@briandipalma as far as I can see, it only adds unnecessary complexity.

What if you added a setTimeout of 0?

That seems like a dirty trick to prevent I/O starvation. The question is, what actual use case would that solve?

[briandipalma]

I can see a case where you want to wrap something in a Promise so it can be consumed by an API that only consumes Promises.

[UltCombo]

@briandipalma makes sense. In that case, resolving synchronously shouldn't be a problem I believe.

[domenic]

In that case you should use Promise.resolve() on the synchronously-retrieved result before passing it to such a (very poorly designed) API.

Don't make synchronous things asynchronous for no reason.

[johnjbarton]

PR #1266 takes steps towards API improvement.

[johnjbarton]

PR #1278 removes the async calls.

[UltCombo]

This would be a nice 1.0 API, IMHO.

[johnjbarton]

Almost all of our options configure our compiler for an application. That is they are intended to be set once and used for every compilation in a given app or environment.

On glaring exception is script; this option is a command line switch applying only to the next file in the line. In effect script is a global flag mutating our API.

I think this option should be removed and the switch should be placed in our API, either with a boolean argument:

   function compile(src, options, sourceName, outputName, parseAsScript = false);
   Compiler.parse(content, sourceName, parseAsScript = false);

or as a separate call delegating to a call with a switch:

   function compileAsModule(src, options, sourceName, outputName); 
   function compileAsScript(src, options, sourceName, outputName); 
   Compiler.parseAsModule(content, sourceName);
   Compiler.parseAsScript(content, sourceName);

(As discussed else where a differ issue is to group the minor arguments into an object:

  function compileAsModule(src, {
    options = {}, 
    sourceName = '<compileAsModuleInput>', 
    outputName = '<compileAsModuleOuttput>'
  });