Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define a new node type as a chunk of code to output verbatim #73

Closed
drslump opened this issue Oct 17, 2012 · 14 comments
Closed

Define a new node type as a chunk of code to output verbatim #73

drslump opened this issue Oct 17, 2012 · 14 comments

Comments

@drslump
Copy link
Contributor

drslump commented Oct 17, 2012

Currently I don't see any way to pass a chunk of code to be outputted just as is in the generated source. The use case is when generating an AST from a language that allows to define chunks of code to be placed verbatim in the generated javascript, much like CoffeeScript's backtick notation.

It could work something like this:

{
    type: 'CodeExpression',
    code: 'window.location.href = "http://google.com"'
}

The generator would output the contents of the code with only the following processing performed:

  • Indent first line to the current level if it belongs to ExpressionStatement
  • Indent additional lines to the current level

Since it's impossible to know the precedence order of that node, it should always be assumed that it's very low (Assignment), wrapping it with parens if used in a binary or member expression for example.

@Constellation
Copy link
Member

I think we can handle it by parsing it by Esprima and injecting produced AST to given tree before passing AST to Escodegen, right?

@michaelficarra:
How does CoffeeScriptRedux handle it?

@drslump
Copy link
Contributor Author

drslump commented Oct 17, 2012

I would prefer to don't have a dependency on Esprima (or any other Javascript parser). I chose Escodegen in the first place because it focuses on doing just one thing :)

The solution I'm using right now is to convert those chunks to CallExpression('eval', [ LiteralExpression(code) ]), but this obviously means an impact on the runtime performance, which is most of the time the main reason to embed code this way.

@michaelficarra
Copy link
Member

@Constellation: CoffeeScriptRedux does what @drslump describes. That feature of CoffeeScript is highly discouraged, so we don't worry about the performance costs. The method you describe (parsing it with Esprima) was considered a few times, but we would rather avoid that dependency and just produce an eval. Another option to consider is marking esprima as an optional dependency, only requiring it if you see one of these nodes. It's worth reading the related issue, michaelficarra/CoffeeScriptRedux#71.

@Constellation
Copy link
Member

@michaelficarra:

That feature of CoffeeScript is highly discouraged, so we don't worry about the performance costs. The method you describe (parsing it with Esprima) was considered a few times, but we would rather avoid that dependency and just produce an eval.

I see. It's reasonable decision.

@drslump:
Escodegen uses Mozilla JS AST because it considers this representation as Intermediate Representation. By using this IR, we can apply a lot of modules using this IR to your AST, for example, Esmangle can analyze / optimize / minify this AST. And at the same time, this IR keeps Escodegen simple. Escodegen's work is only generating ECMAScript code from Mozilla JS AST, not parsing code & transforming tree.

If we introduce CodeExpression specially, we must throw this benefit. Modules cannot analyze CodeExpression because of lack of structure.

So I would prefer these ways, parsing it by Esprima or transforming it to eval call.
Of cource, eval call makes runtime preformance regression and prevents optimization.
So if you would not like to have dependency on Esprima, you can take eval call. But if you think this performance regression isn't negligible, I recommend to use Esprima. Esprima is very simple and well-modularized, so I think you would like it :)

@michaelficarra
Copy link
Member

@Constellation: I think that's a good decision. Let the consumers of escodegen add esprima as a dependency if that's the functionality they need.

@drslump
Copy link
Contributor Author

drslump commented Oct 17, 2012

@Constellation:

Unfortunately I cannot easily include Esprima since my compiler runs on .Net, executing escodegen with a Javascript interpreter, which is not the fastest thing on earth, while I work on a native code generation implementation.

I understand your concerns but to IMHO using a code generation library means I should be given as much control as possible about what to place in the output. Even if the resulting AST would not be interoperable with other tools, I might choose to generate one version of the AST for escodegen and a standard one using calls to eval.
With that in mind, perhaps I could present a different alternative which would keep the AST compatible with Mozilla's while at the same time allowing more control when using escodegen.

Instead of defining a new node type, expression node types could support an annotation (similarly to how comments are being implemented right now), which would tell escodegen to use the code string defined there instead of traversing the node.

CallExpression:
  callee: Identifier('eval')
  arguments: [ Literal('evaled.code = literal.code') ]
  x-replace: 'evaled.code == literal.code'

When traversing the AST, if the node contains an x-replace annotation it will include that in the output instead of processing the node.

@michaelficarra
Copy link
Member

@drslump: How about using the raw value? We could enhance escodegen to use raw values of any node when it exists instead of just for literals.

@drslump
Copy link
Contributor Author

drslump commented Oct 17, 2012

@michaelficarra: +1

@Constellation
Copy link
Member

Now Escodegen checks parse(raw) == tree structurally equal. Because when we modify tree but forget to delete raw value, this becomes difficult bug to find.
Do you have any ideas to implement above idea properly?

@drslump
Copy link
Contributor Author

drslump commented Oct 17, 2012

Since it seems that raw has some special meaning in Esprima-Escodegen perhaps it's better to use a completely separate annotation. The implementation could be as simple as the following (in generateExpression, above the switch):

if (typeof expr['x-replace'] === 'string') {
  result = expr['x-replace'].split(/\r\n|\n/)
  for (var i=1; i<result.length; i++)
    result[i] = newline + base + result[i];

  result = parenthesize(result, Precedence.Assignment, precedence)
  return toSourceNode(result, expr)
}

@Constellation
Copy link
Member

Nice concept. And I would prefer to pass special name with option, like this,

escodegen.generate(AST, { verbatim: 'x-replace' })

And we should note, if verbatim option is used, we cannot ensure result code is valid JavaScript and modules treating Mozilla AST may not recognize this(for example, Esmangle will be broken when this option is used).

@Constellation Constellation reopened this Oct 20, 2012
@Constellation
Copy link
Member

In the meantime, we enable verbatim of Expression. So indentation is not considered.

@Constellation
Copy link
Member

verbatim options is committed in 582483f 382633f

@Constellation
Copy link
Member

Thanks, I've merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants