dillon1000/react
Commit
Browse files [RFC] Stabilize naming of promoted temporaries
When the compiler promotes temporary values to named variables, we currently
eagerly assign a name using the temporary's IdentifierId. This means that we're
sort of stuck with this name later in compilation, and RenameVariables can't be
100% sure whether a 't0' variable is a temporary or not. As a result, the names
of these promoted temporaries is influenced by how many temporaries we happened
to create during compilation (and what the next available identifier id was),
making them fluctuate more as we iterate on the compiler.
This is an RFC for showing how we can stabilize these names. The key elements:
* Distinguish promoted temporaries from other named identifiers. Here we use a
hack, naming them starting with '#t' or '#T', since '#' isn't a valid identifier
starting point. This lets us keep all of our logic that looks for non-null
identifiers names to distinguish named/unnamed, while also distinguishing real
names from generated names (if this was Rust, we'd use an Enum and have a
"isNamed()" method on it that was true for real/temporary names and false
otherwise)
* In RenameVariables, detect generated names and fall back to generating the
next available `tN`-style name (or `TN` for JSX tags).
* To reduce thrash overall, RenameVariables no longer keeps a global "next id"
value that uses to distinguish all conflicting identifiers, instead we restart
at 0 whenever we find a conflict, and keep bumping until we find a free name.
Thus if both `foo` and `bar` had conflicts, we previously would end up with
`foo$0` and `bar$1` as the deduped names, but now will end up with `foo$0` and
`bar$0`.
## RFC
I'm open to feedback on the approach. Two main questions:
* How to annotate promoted temporaries. The most type-safe option is to change
`Identifier.name` to be a union of `{kind: 'named', value: string} | `{kind:
'promoted', value: string} | `{kind: 'temporary'}` though TS then wouldn't allow
`identifier.name.value` (even as nullable) since it doesn't exist on one of the
variants. Maybe we could type the temporary one as `{kind: 'temporary', value?:
null}` so the value has to be null but you can always access that property?
* ?? Other concerns about the approach? We could keep the global
auto-incrementing id rather than attempting to reset to 0 for each conflict.Changed paths76 files
First-parent comparisoncompiler/packages/babel-plugin-react-forget/src/Inference/InlineImmediatelyInvokedFunctionExpressions.ts ModifiedM compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts ModifiedM compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PromoteUsedTemporaries.ts ModifiedM compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PropagateEarlyReturns.ts ModifiedM compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/RenameVariables.ts ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/allocating-primitive-as-dep-nested-scope.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/array-access-assignment.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/array-at-effect.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-import.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-namespace-import.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug.useMemo-deps-array-not-cleared.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/call-with-independently-memoizable-arg.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/conditional-early-return.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/createElement-freeze.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructuring-mixed-scope-declarations-and-locals.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/drop-methodcall-usememo.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-nested-early-return-within-reactive-scope.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-no-declarations-reassignments-dependencies.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-within-reactive-scope.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/escape-analysis-destructured-rest-element.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/hooks-with-prefix.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/iife-return-modified-later-logical.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/iife-return-modified-later-phi.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/iife-return-modified-later.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/inner-memo-value-not-promoted-to-outer-scope-dynamic.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/jsx-tag-evaluation-order-non-global.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/jsx-tag-evaluation-order.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/nested-function-with-param-as-captured-dep.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/noAlias-filter-on-array-prop.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/obj-literal-cached-in-if-else.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/object-shorthand-method-2.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/partial-early-return-within-reactive-scope.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/primitive-as-dep-nested-scope.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reassignment-conditional.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reassignment.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/regexp-literal.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/remove-memoization-kitchen-sink.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-dont-memoize-array-with-mutable-map-after-hook.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-hoisting-variable-collision.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-reassign-to-variable-without-mutable-range.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-renaming-conflicting-decls.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-undefined-expression-of-jsxexpressioncontainer.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rules-of-hooks/allow-locals-named-like-hooks.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rules-of-hooks/allow-props-named-like-hooks.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/sequential-destructuring-assignment-to-scope-declarations.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/sequential-destructuring-both-mixed-local-and-scope-declaration.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/simple.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ssa-property-alias-if.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/switch-non-final-default.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/todo.unnecessary-lambda-memoization.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/transitive-freeze-function-expressions.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-try-value-modified-in-catch.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-with-catch-param.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-with-return.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-within-mutable-range.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/type-annotations/todo_type-annotations-props.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/unused-object-element-with-rest.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-if-else-multiple-return.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-independently-memoizeable.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-inlining-block-return.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-inverted-if.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-labeled-statement-unconditional-return.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-logical.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-mabye-modified-free-variable-dont-preserve-memoization-guarantees.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-mabye-modified-free-variable-preserve-memoization-guarantees.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-maybe-modified-later-dont-preserve-memoization-guarantees.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-maybe-modified-later-preserve-memoization-guarantees.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-multiple-if-else.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-simple.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-switch-no-fallthrough.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-switch-return.expect.md ModifiedM compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/validate-no-set-state-in-render-uncalled-function-with-mutable-range-is-valid.expect.md ModifiedPatch
Files changed
Rendering syntax-highlighted changes…