Add walk support #87
Add walk support #87
Conversation
|
Hmm, are many consumers really using built-in walk instead of estraverse or similar? |
|
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 acornjs/acorn#656 (comment). |
|
@RReverser It's quite confusing when you switch to It was surprising to me that I believe, since a walker is a part of |
walk.js
Outdated
| base.JSXOpeningFragment = base.JSXEmptyExpression = base.JSXIdentifier = base.JSXText = base.JSXClosingFragment = base.Identifier; | ||
| base.JSXAttribute = function(node, st, c) { | ||
| c(node.name, st); | ||
| c(node.value, st); |
There was a problem hiding this comment.
node.value can be null – if (node.value) is needed here
walk.js
Outdated
| for (var i = 0; i < node.children.length; ++i) { | ||
| c(node.children[i], st); | ||
| } | ||
| c(node.closingElement, st); |
There was a problem hiding this comment.
node.closingElement can be null – if (node.closingElement) is needed here
| for (var i = 0; i < node.children.length; ++i) { | ||
| c(node.children[i], st); | ||
| } | ||
| c(node.closingFragment, st); |
There was a problem hiding this comment.
node.closingFragment can be null – if (node.closingFragment) is needed here
There was a problem hiding this comment.
Can you give me an example for this case?
There was a problem hiding this comment.
According regular rules <React.Fragment /> is a valid statement, even looks odd. You also can use <React.Fragment children={props.children} /> or <React.fragment {...props} /> for some reasons.
There was a problem hiding this comment.
On a syntax level, that's just JSXElements. A fragment in terms of syntax is something like <></>.
There was a problem hiding this comment.
I see, yep, probably checking is not required here, since anonymous fragments can't have attributes.
|
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! |
|
Thanks for reminding me, I actually forgot this was my PR. I addressed the feedback. |
|
Would love to see this landed and released soon! @RReverser |
|
Hopeful for this to land soon. It would be a big fix for i18next-scanner. |
|
Closing this in favor of sderosiaux/acorn-jsx-walk#3. |
This is on top of #86.