Restore traversal context after enter / traverse

[JLHwung]

Q	A
Fixed Issues?	Fixes #13801
Patch: Bug Fix?	Y
Major: Breaking Change?	Maybe.
Minor: New Feature?
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?
License	MIT

In this PR we restores the context after call("enter") and traverse.node. The current AST operations we provided did not guarantee that the traversal context is perserved, let alone users may even change the associated context. The traversal context is crucial to sub-traverse in order to isolate visitors / traverse states from the root traverse.

See also #13801 (comment) for how the leaked traversal context causes confusing errors.

I add a benchmark case to access the performance impact of this PR. The benchmark is a worst case scenario because context is changed in every node visitor. Compared to 7.15.4, Babel traverse in this PR is 10% slower. In the real world most context change happen in sub traverse, preserving context will ensure global visitors are not invoked in sub traverse so the real world performance should be better than this.

Benchmark Results

baseline 256 empty statements: 6_354 ops/sec ±1.76% (0.157ms)
baseline 512 empty statements: 3_287 ops/sec ±0.83% (0.304ms)
baseline 1024 empty statements: 1_615 ops/sec ±0.92% (0.619ms)
baseline 2048 empty statements: 795 ops/sec ±1.91% (1.259ms)
baseline mutating context 256 empty statements: 6_570 ops/sec ±0.94% (0.152ms)
baseline mutating context 512 empty statements: 3_237 ops/sec ±1.4% (0.309ms)
baseline mutating context 1024 empty statements: 1_635 ops/sec ±0.91% (0.612ms)
baseline mutating context 2048 empty statements: 811 ops/sec ±1.16% (1.233ms)
current 256 empty statements: 5_151 ops/sec ±19.24% (0.194ms)
current 512 empty statements: 2_843 ops/sec ±1.74% (0.352ms)
current 1024 empty statements: 1_424 ops/sec ±0.96% (0.702ms)
current 2048 empty statements: 706 ops/sec ±2.6% (1.417ms)
current mutating context 256 empty statements: 6_270 ops/sec ±0.62% (0.159ms)
current mutating context 512 empty statements: 3_121 ops/sec ±0.98% (0.32ms)
current mutating context 1024 empty statements: 1_553 ops/sec ±0.91% (0.644ms)
current mutating context 2048 empty statements: 758 ops/sec ±1.76% (1.318ms)

[babel-bot]

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/49081/

[codesandbox-ci]

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 f11e12d:

Sandbox	Source
babel-repl-custom-plugin	Configuration
babel-plugin-multi-config	Configuration

[JLHwung]

If this.enter("enter") is false, either fns is empty (no visitors), or all the visitors does not change the traverse flags. If fns is empty, then this.shouldSkip would not be changed at all. If all the visitors do not change traversal flags, then this.shouldSkip should equals to this.shouldSkip before enter hook is called. Therefore the second this.shouldSkip check is redundant.

[nicolo-ribaudo]

Does this also fix #12631?

[JLHwung]

Does this also fix #12631?

Thanks for the reminder. I check out the reproduction repo: The traversal context does change after .call("enter"). After I apply the fix to the next-bundled babel.js, the context is correctly restored, but the error d(...) is not a function can still be reproduced.

[danieltroger]

Is it intentional that this function is no longer invoked?

[JLHwung]

Kind of. Personally I prefer the minimal AST structure that can reproduce the bug. In this case we are not testing the runtime semantics so the simpler the input code is, the easier to reason about what is going on. You can verify that the original code example also works on the PR REPL.

[danieltroger]

Very epic thanks for explaining

[nicolo-ribaudo]

Could we also benchmark, for example, compiling https://github.com/babel/babel/blob/main/packages/babel-parser/src/parser/expression.js with @babel/preset-env + @babel/preset-flow to see the real world perf diff?

[JLHwung]

I added a real-world benchmark, the current PR is 3% (not statistically significant) slower than main branch. The minimal sample size for benchmark is 100.

baseline babel-parser-expression.txt: 5.93 ops/sec ±2.97% (169ms)
current babel-parser-expression.txt: 5.78 ops/sec ±3.51% (173ms)