Mark static block as FunctionParent #13832
Conversation
JLHwung
added
PR: Bug Fix 🐛
|
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 c3a7fd9:
|
| expect(switchStatement.scope.hasOwnBinding("foo")).toBe(true); | ||
| }); | ||
| }); | ||
| //todo: decide whether these statements should be scopeable and blockParent |
There was a problem hiding this comment.
I don't think WhileStatement and DoWhileStatement should be marked as Scopeable and BlockParent: The expression in while ( ) can not introduce variable declarations.
We can remove them in Babel 8.
Ahh TIL
do
var x = 1
while (x);is valid. I will add more tests.
There was a problem hiding this comment.
The bindings should be registered in the block inside the loop.
There was a problem hiding this comment.
Yeah we already register let bindings in the block statements. since
do
let x;
while (0)
is not allowed. I still think we can remove WhileStatement and DoWhileStatement from Scopeable and BlockParent.
There was a problem hiding this comment.
It should match whatever if, for-in, for-of and with are.
There was a problem hiding this comment.
Yeah. We don't mark IfStatement as Scopeable and BlockParent because all the introduced let declarations are from its underneath BlockStatement. But we do mark ForIn/ForOf because they can introduce let declarations other than from the BlockStatement.
| const staticBlock = getPath("(class { static { var foo; } })", { | ||
| plugins: ["classStaticBlock"], | ||
| }).get("body.0.expression.body.body.0"); | ||
| expect(staticBlock.scope.hasOwnBinding("foo")).toBe(true); |
There was a problem hiding this comment.
The old behavior was that var foo was registered as an own binding of the program scope? If so, we might want to also test that it's not registered in the global scope.
There was a problem hiding this comment.
Yes, the old behavior is to register in upper functionParent scope / program scope.
we might want to also test that it's not registered in the global scope.
Well we can add a general test that the same binding should not be registered twice in different scope, is that true?
There was a problem hiding this comment.
Oh right, testing that a binding is in one scope is effectively already testing that it's not in the other one.
JLHwung
force-pushed
the
mark-static-block-as-function-parent
branch
from
32e4d81
to
f6b5fae
Compare
| @@ -206,7 +206,7 @@ const aliasDescriptions = { | |||
| ForXStatement: | |||
| "A cover of [ForInStatements and ForOfStatements](https://tc39.es/ecma262/#sec-for-in-and-for-of-statements).", | |||
| Function: | |||
| "A cover of functions and [method](#method)s, the must have `body` and `params`. Note: `Function` is different to `FunctionParent`.", | |||
| "A cover of functions and [method](#method)s, the must have `body` and `params`. Note: `Function` is different to `FunctionParent`. For example, a `StaticBlock` is a `FunctionParent` but not `Function`.", | |||
| FunctionParent: | |||
There was a problem hiding this comment.
Maybe we should rename FunctionParent to VarHoistScope?
There was a problem hiding this comment.
I am good with renaming and adding alias for backward compatibility. The FunctionParent is named v.s. BlockParent. If we pick VarHoistScope for FunctionParent, then we should also pick one for BlockParent: Maybe LexicalScope?
We also have Scope#getFunctionParent and Scope#getBlockParent which returns the scope's path's someParent's scope. They will also have to be renamed.
Related: By definition I would like to add Program to FunctionParent too. The Program is a FunctionParent in Babel 6 but we mark it as BlockParent since Babel 7: An AST node could be both BlockParent and FunctionParent, though: Precisely FunctionParent is a subset of BlockParent.
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/49135/ |
github-actions
bot
added
the
outdated
In this PR we mark
StaticBlockas aFunctionParent, it was overlooked in #12079. We also enumerated tests forBlockParentandFunctionParent.