Use NodePath#hub as part of the paths cache key#15725
Use NodePath#hub as part of the paths cache key#15725nicolo-ribaudo merged 5 commits intobabel:mainfrom
NodePath#hub as part of the paths cache key#15725Conversation
nicolo-ribaudo
added
the
PR: Bug Fix 🐛
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/54782/ |
|
I'm very confused by the "old Babel" e2e failure. That test is using EDIT: The unit test was actually using both old and new at the same time, and there are some problems with mixing traverse versions. Fixed by #14794. |
nicolo-ribaudo
force-pushed
the
fix-hub-cach
branch
from
e71e957 to
af1a93e
Compare
nicolo-ribaudo
changed the title
NodePath#hub as part of the cache keyNodePath#hub as part of the paths cache key
liuxingbaoyu
left a comment
liuxingbaoyu
left a comment
There was a problem hiding this comment.
Would it be better for us to use || instead of ?? in this PR?
|
Posting this here to save some headaches to others If anyone gets the error Cannot read properties of undefined (reading 'addHelper')
at Scope.toArray (@babel/traverse/lib/scope/index.js:464:36)
at getSpreadLiteral (@babel/plugin-transform-spread/lib/index.js:21:20)When building after upgrading babel from 7.22.6 to 7.22.7 Make sure to switch from |
This reverts commit abb5a7c.
github-actions
bot
added
the
outdated
5 participants
This fixes #6437, implementing option 2. I solved the problem of "we call
traversewith no hub" by adding support for traversing a node visiting also the top-level node itself, so that we can directly traverse theProgramNodePath that already has the hub.We might want to, at some point, expose that top-level traversal as an official API, such as
NodePath#traverseSelf, but I'm intentionally keeping it under the radar now (even if technically the new capability is accessible from the third-party code).I don't know if this fixes or not #12570, because it looks like that issue has already been fixed on
main?cc @robhogan this fixes the underlying issue of facebook/metro#906