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

Support of es2016's decorators #116

Closed
AugustinLF opened this issue Jul 6, 2015 · 6 comments
Closed

Support of es2016's decorators #116

AugustinLF opened this issue Jul 6, 2015 · 6 comments

Comments

@AugustinLF
Copy link

I tried using recast on a file containing es2016 decorators. Of course, it failed (unexpected token illegal), since, based on what I understood, this syntax is not an ast type (I had the same problem with the es2016 bind syntax). Considering that the async function type exist, I guess that it could be possible to add the support to decorators (both are es2016 proposals).

I'm willing to work on this issue, however, I struggle to see what would be the correct way to start. I would actually be fine to add a partial support to this feature, considering that what I need, is to be able to parse a file without breaking, not to add any decorator. Do you have any tip you could share with me?

@benjamn
Copy link
Owner

benjamn commented Jul 6, 2015

Fortunately, in this case, I think the only tricky part is the parsing.

When you parse a string of code with recast.parse, you can provide your own parser (the default parser is esprima-fb, which definitely doesn't know about the latest features):

var ast = recast.parse(source, {
  esprima: require("babel-core")
});

Any object with a .parse method like require("esprima-fb").parse should work, though I've only tested it with Esprima, Acorn, and the Babel parser.

As of recently, recast.print knows about decorators and bind syntax, which means you can create new Decorator AST nodes, or modify existing ones, and recast.print will be able to generate new code when necessary. (Often, recast.print just copies code from the original source, without understanding its syntax, but it's always good for the pretty printer to know about all the AST types you might use, just in case they have to be reprinted.)

In general, I am totally happy to add experimental syntax to the pretty printer, even when it conflicts with existing syntax. For example, the ImportSpecifier case handles two completely different kinds of nodes, with different fields, and that's fine with me!

The ast-types library (which powers recast.visit), also recently learned about Decorator and BindExpression nodes, so you should now be able to do something like

// Note: recast.visit is the same as require("ast-types").visit.
recast.visit(ast, {
  visitDecorator: function(path) {
    var node = path.node;
    // ...
    this.traverse(path);
  }
});

So in this case, there might not be too much work for you to do (sorry? haha), though I would be very happy to hear about any problems you encounter, since these features are still pretty new/untested.

@benjamn
Copy link
Owner

benjamn commented Jul 6, 2015

I originally wrote that reply thinking it was a recast issue rather than an ast-types issue, so please pardon the weird wording about ast-types, as if it was something you might not know about, when clearly you do know about it already!

@AugustinLF
Copy link
Author

Oh, ok, thanks a lot! I thought that ast-type didn't support decorators, since I couldn't find the definition in the es7.js file. using the babel-core parser in recast actually works for me, I don't think I need to do anything specific right now, since I just wanted to work in a file having using this syntax. I'll close this issue, however, if you don't mind, I'll make a pull request to update the readme of recast, to explain the possibility to use others parser, this will avoid you questions like mine :)

@AugustinLF
Copy link
Author

Any estimation about when do you plan to publish the last versions of recast and ast-types to NPM? Because I was trying to figure why it didn't work for me, it's just because the version I had (from npm) didn't work, getting the sources from github worked fine :)

@benjamn
Copy link
Owner

benjamn commented Jul 8, 2015

Though I just published a small update (0.8.2), I wonder if you're still depending on ~0.7? The 0.7 to 0.8 minor version bump was pretty recent, in case you missed it.

@AugustinLF
Copy link
Author

When did you publish the update? Because I was conviced it was still on ~0.7 (as displayed both by npm view and the npm page) a few hours ago. I'll test that tomorrow anyway :) And if you did indeed update that today, thanks a lot!

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

2 participants