feat(traverse): record current block scope

[overlookmotel]

Record "block" scope ID along with "hoist" scope ID in Traverse.

"Block" scope is the scope where a let statement would be inserted above current position in AST.

Block scope and current scope differ from each other inside classes. For example, if want to create a let temp var for foo() or bar() in example below, should use block scope not current scope. Current scope is the class itself, but the let statement will be inserted outside the class.

class C {
  [foo()]: bar();
}

All transforms which create let bindings should use block scope not current scope. We should add VarDeclarationsStore::insert_let method which uses block scope, to accompany insert_var (which uses hoist scope).

This is implemented in a rather hacky way, and we should improve it later. Notably, we're not considering for statements as block scopes because we currently have no way to insert let statements into them if they don't have a body block.

[overlookmotel]

• chore(transformer): enable class-properties plugin #7750
• refactor(transformer/class-properties): rename file #8036
• fix(transformer/class-properties): replace this and class name in static blocks #8035
• fix(transformer/class-properties): run other transforms on static properties, static blocks, and computed keys #7982
• feat(traverse): record current block scope #8007 👈 (View in Graphite)
• refactor(transformer/class-properties): do not pass ScopeId into insert_instance_inits #8001
• perf(ast): move_expression and move_statement produce dummy with no span #7995
• feat(transformer/class-properties): exit faster from super replacement visitor #8028
• refactor(traverse)!: rename Ancestor::is_via_* methods to is_parent_of_* #8031
• main

---

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• 0-merge - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[codspeed-hq]

CodSpeed Performance Report

Merging #8007 will degrade performances by 3.26%

_{Comparing 12-19-feat_traverse_record_current_block_scope (6b6444b) with main (059a5dd)}

Summary

❌ 1 regressions
✅ 28 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	main	12-19-feat_traverse_record_current_block_scope	Change
❌	semantic[cal.com.tsx]	27.1 ms	28 ms	-3.26%

[Dunqing]

I have taken a look at the whole stack. From the usages, it seems we can get current_block_scope by looking up the parent scope of current_scope_id because when you want to use current_block_scope, you are always at the class scope, so you only need to look up once and store it somewhere. That I understand correctly?

To clarify, I am not rejecting this API at all; on the contrary, I am happy to add various APIs in Traverse and TransformerCtx to solve annoying duplicating code. So If you find that can be fixed or refactored in other places by it, tell me!

[overlookmotel]

it seems we can get current_block_scope by looking up the parent scope of current_scope_id because when you want to use current_block_scope, you are always at the class scope, so you only need to look up once and store it somewhere. That I understand correctly?

That's what I thought but actually no!

This gem was in one of Babel's tests:

class Outer extends class Inner {
  [foo()] = 123;
} {}

So when you're in the inner class, the parent scope is not where a let binding gets inserted - the parent scope is the scope of the outer class!

[Dunqing]

it seems we can get current_block_scope by looking up the parent scope of current_scope_id because when you want to use current_block_scope, you are always at the class scope, so you only need to look up once and store it somewhere. That I understand correctly?

That's what I thought but actually no!

This gem was in one of Babel's tests:

class Outer extends class Inner {
  [foo()] = 123;
} {}

So when you're in the inner class, the parent scope is not where a let binding gets inserted - the parent scope is the scope of the outer class!

Aha, I see the problem, fu** JavaScript!

[graphite-app]

Merge activity

• Dec 19, 10:50 PM EST: A user added this pull request to the Graphite merge queue.
• Dec 19, 10:59 PM EST: A user merged this pull request with the Graphite merge queue.