search

acornjs / acorn-jsx

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

Add walk support #87

Closed adrianheine closed 5 years ago

adrianheine commented 6 years ago

This is on top of #86.

RReverser commented 6 years ago

Hmm, are many consumers really using built-in walk instead of estraverse or similar?

adrianheine commented 6 years ago

I'm planning on using it in bublé, but I can ship it myself. In general, I don't know what the future for acorn walk is, marijn and I had a quick talk about it in https://github.com/acornjs/acorn/issues/656#issuecomment-363814147.

lahmatiy commented 6 years ago

@RReverser It's quite confusing when you switch to acorn-jsx, attempt to walk through AST following readme but get the error:

TypeError: base[type] is not a function

It was surprising to me that acorn-jsx doesn't extend the walker when it can, and I need to use "estraverse or similar" with no mention about it in acorn or acorn-jsx readmes.

I believe, since a walker is a part of acorn it should be extended by acorn-jsx as well.

beheh commented 6 years ago

Looking forward to seeing this landed and released! I'd like to contribute using this in the i18next-scanner project instead of the unmaintained acorn-jsx-walk. So far all the tests in i18next-scanner pass after switching to this project/fork and adding the conditionals as described by lahmatiy.

@adrianheine do you think you could get the minor changes in this PR, or @lahmatiy just manually add them and do a release? Thanks!

adrianheine commented 6 years ago

Thanks for reminding me, I actually forgot this was my PR. I addressed the feedback.

beheh commented 6 years ago

Would love to see this landed and released soon! @RReverser

chacestew commented 5 years ago

Hopeful for this to land soon. It would be a big fix for i18next-scanner.

adrianheine commented 5 years ago

Closing this in favor of sderosiaux/acorn-jsx-walk#3.