fix(traverse): set the correct context when getting multiple NodePath#12848
fix(traverse): set the correct context when getting multiple NodePath#12848fedeci wants to merge 3 commits intobabel:mainfrom
NodePath#12848Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/47769/ |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit c7db2a6:
|
nicolo-ribaudo
added
pkg: traverse
PR: Bug Fix 🐛
|
Could you check if this also fixes #12631, which is another context-related bug? |
|
I applied the same changes to |
| container: container, | ||
| key: i, | ||
| }).setContext(context); | ||
| }).setContext(v.context); |
There was a problem hiding this comment.
I think v.context should be supplied as a fallback of context.
There was a problem hiding this comment.
Here the context defaults to true, so if no specific context is passed it will be used the one of the current NodePath (It may not be the right one).
Because of this we can't fallback since context is always defined when calling #get().
babel/packages/babel-traverse/src/path/family.ts
Lines 188 to 202 in 468a43c
There was a problem hiding this comment.
In this case we can use v.context when context is true, otherwise when a context is indeed passed as TraversalContext, we should respect the input, though.
fedeci
force-pushed
the
regression-12570
branch
from
468a43c to
404539f
Compare
| this: NodePath<T>, | ||
| key: string, | ||
| context?: TraversalContext, | ||
| context?: true | TraversalContext, |
There was a problem hiding this comment.
Just a couple questions.
Since this method is only called from #get() this shouldn't be marked as optional because it will default to true when no value is provided. Right?
Also this is not expected to be called from third party plugins, then it shouldn't be exported?
| this: NodePath, | ||
| parts: string[], | ||
| context?: TraversalContext, | ||
| context?: true | TraversalContext, |
fedeci
force-pushed
the
regression-12570
branch
from
404539f to
c7db2a6
Compare
| container: container, | ||
| key: i, | ||
| }).setContext(context); | ||
| }).setContext(isValidContext ? context : v.context); |
There was a problem hiding this comment.
v.context is always undefined because v is a Node not a NodePath.
github-actions
bot
added
the
outdated
4 participants
Uh oh!
There was an error while loading. Please reload this page.