acornjs / acorn-jsx

Alternative, faster React.js JSX parser
MIT License
648 stars 72 forks source link

Port to Acorn 6.0 plugin interface #89

Closed marijnh closed 5 years ago

marijnh commented 6 years ago

This is a proof-of-concept porting the plugin to the (unreleased, proposed) plugin API for Acorn 6. I'd like to get your opinion on whether this is a reasonable way forward (both for the Acorn API and for this plugin.) Some issues that might be contentious are:

The structure of the code itself is pretty much identical to how it used to be, except that I'm not mutating the objects in the main library, but keeping token types and contexts local, and putting new methods in the mixin class.

marijnh commented 6 years ago

(To test, you'll have to symlink in a build of the current Acorn master branch)

marijnh commented 5 years ago

6.0.0 has been released now. If you want me to add something like a Bublé pass, let me know!

RReverser commented 5 years ago

Sorry, last few days have been busy. I'll review early next week.

marijnh commented 5 years ago

👋

RReverser commented 5 years ago

Generally seems good to me, thanks for your work!

Aside from ES6 (which doesn't worry me much at this point tbh, if someone needs to, they can transpile) one thing that comes to mind is removal of index.js. On one hand, this makes consumer think about composing all the plugins together which is a good thing generally, but on another I don't think there is much harm in continuing providing a prepared instance of Acorn with JSX support? Not sure yet.

RReverser commented 5 years ago

Ok, I suppose it's fine...

RReverser commented 5 years ago

Published as 5.0.0.

marijnh commented 5 years ago

Thanks!

We could have provided an extended Parser instance in the public API, to remove the need for people to require two packages. But I kind of like that Acorn can be a peer-dependency this way, which removes issues around npm or yarn installing/loading multiple versions of Acorn.