-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Set correct path.context
un push/unshiftContainer
#12394
Set correct path.context
un push/unshiftContainer
#12394
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/33201/ |
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 bdb738c:
|
@@ -78,6 +98,26 @@ describe("modification", function () { | |||
}, | |||
}); | |||
}); | |||
|
|||
it("should set the correct path.context", function () { | |||
expect.assertions(2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably add this to all the tests where we expect
inside a visitor, just in case the visitor doesn't get called!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference only, this PR fixes regression introduced in #12331. The fix here is complete as it covers all the NodePath.get
usage in @babel/traverse
.
Both the test cases in this PR are failing without the
.setContext()
calls. The first one with "cannot read.skipKeys
ofnull
(becausepushContianer
uses aNodePath
not in the cache) and the second one with["consequent"] != []
(because it re-uses the options from the inner traversal).path.opts
in this PR is a direct side effect of not having the correctcontext
, and it can cause problem when requeuing (since paths could be requeued in an "inactive" context of a nestedpath.traverse
finished call).