diff --git a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts index 3025709be2..d1ddf4fd0f 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts @@ -348,7 +348,7 @@ function* runWithEnvironment( value: reactiveFunction, }); - renameVariables(reactiveFunction); + const uniqueIdentifiers = renameVariables(reactiveFunction); yield log({ kind: "reactive", name: "RenameVariables", @@ -373,7 +373,11 @@ function* runWithEnvironment( validatePreservedManualMemoization(reactiveFunction); } - const ast = codegenFunction(reactiveFunction, filename).unwrap(); + const ast = codegenFunction( + reactiveFunction, + uniqueIdentifiers, + filename + ).unwrap(); yield log({ kind: "ast", name: "Codegen", value: ast }); /** diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts index 5a156f30b1..f76613b641 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts @@ -970,9 +970,9 @@ export type Identifier = { type: Type; }; -export type IdentifierName = - | { kind: "named"; value: ValidIdentifierName } - | { kind: "promoted"; value: string }; +export type IdentifierName = ValidatedIdentifier | PromotedIdentifier; +export type ValidatedIdentifier = { kind: "named"; value: ValidIdentifierName }; +export type PromotedIdentifier = { kind: "promoted"; value: string }; /** * Simulated opaque type for identifier names to ensure values can only be created @@ -988,7 +988,7 @@ export type ValidIdentifierName = string & { * identifier names: only call this method for identifier names that appear in the * original source code. */ -export function makeIdentifierName(name: string): IdentifierName { +export function makeIdentifierName(name: string): ValidatedIdentifier { CompilerError.invariant(t.isValidIdentifier(name), { reason: `Expected a valid identifier name`, loc: GeneratedSource, diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts index 90fbecc0e8..f5e83aa2fb 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -31,7 +31,9 @@ import { ReactiveValue, SourceLocation, SpreadPattern, + ValidIdentifierName, getHookKind, + makeIdentifierName, } from "../HIR/HIR"; import { printPlace } from "../HIR/PrintHIR"; import { eachPatternOperand } from "../HIR/visitors"; @@ -68,9 +70,15 @@ export type CodegenFunction = { export function codegenFunction( fn: ReactiveFunction, + uniqueIdentifiers: Set, filename: string | null ): Result { - const cx = new Context(fn.env, fn.id ?? "[[ anonymous ]]", null); + const cx = new Context( + fn.env, + fn.id ?? "[[ anonymous ]]", + uniqueIdentifiers, + null + ); const compileResult = codegenReactiveFunction(cx, fn); if (compileResult.isErr()) { return compileResult; @@ -95,7 +103,7 @@ export function codegenFunction( compiled.body.body.unshift( t.variableDeclaration("const", [ t.variableDeclarator( - t.identifier("$"), + t.identifier(cx.synthesizeName("$")), t.callExpression(t.identifier("useMemoCache"), [ t.numericLiteral(cacheCount), ]) @@ -215,14 +223,18 @@ class Context { temp: Temporaries; errors: CompilerError = new CompilerError(); objectMethods: Map = new Map(); + uniqueIdentifiers: Set; + synthesizedNames: Map = new Map(); constructor( env: Environment, fnName: string, + uniqueIdentifiers: Set, temporaries: Temporaries | null = null ) { this.env = env; this.fnName = fnName; + this.uniqueIdentifiers = uniqueIdentifiers; this.temp = temporaries !== null ? new Map(temporaries) : new Map(); } get nextCacheIndex(): number { @@ -236,6 +248,21 @@ class Context { hasDeclared(identifier: Identifier): boolean { return this.#declarations.has(identifier.id); } + + synthesizeName(name: string): ValidIdentifierName { + const previous = this.synthesizedNames.get(name); + if (previous !== undefined) { + return previous; + } + let validated = makeIdentifierName(name).value; + let index = 0; + while (this.uniqueIdentifiers.has(validated)) { + validated = makeIdentifierName(`${name}${index++}`).value; + } + this.uniqueIdentifiers.add(validated); + this.synthesizedNames.set(name, validated); + return validated; + } } function codegenBlock(cx: Context, block: ReactiveBlock): t.BlockStatement { @@ -348,12 +375,16 @@ function codegenReactiveScope( changeExpressionComments.push(printDependencyComment(dep)); const comparison = t.binaryExpression( "!==", - t.memberExpression(t.identifier("$"), t.numericLiteral(index), true), + t.memberExpression( + t.identifier(cx.synthesizeName("$")), + t.numericLiteral(index), + true + ), depValue ); if (cx.env.config.enableChangeVariableCodegen) { - const changeIdentifier = t.identifier(`c_${index}`); + const changeIdentifier = t.identifier(cx.synthesizeName(`c_${index}`)); statements.push( t.variableDeclaration("const", [ t.variableDeclarator(changeIdentifier, comparison), @@ -367,7 +398,11 @@ function codegenReactiveScope( t.expressionStatement( t.assignmentExpression( "=", - t.memberExpression(t.identifier("$"), t.numericLiteral(index), true), + t.memberExpression( + t.identifier(cx.synthesizeName("$")), + t.numericLiteral(index), + true + ), depValue ) ) @@ -398,7 +433,11 @@ function codegenReactiveScope( t.expressionStatement( t.assignmentExpression( "=", - t.memberExpression(t.identifier("$"), t.numericLiteral(index), true), + t.memberExpression( + t.identifier(cx.synthesizeName("$")), + t.numericLiteral(index), + true + ), wrapCacheDep(cx, name) ) ) @@ -408,7 +447,11 @@ function codegenReactiveScope( t.assignmentExpression( "=", name, - t.memberExpression(t.identifier("$"), t.numericLiteral(index), true) + t.memberExpression( + t.identifier(cx.synthesizeName("$")), + t.numericLiteral(index), + true + ) ) ) ); @@ -426,7 +469,11 @@ function codegenReactiveScope( t.expressionStatement( t.assignmentExpression( "=", - t.memberExpression(t.identifier("$"), t.numericLiteral(index), true), + t.memberExpression( + t.identifier(cx.synthesizeName("$")), + t.numericLiteral(index), + true + ), wrapCacheDep(cx, name) ) ) @@ -436,7 +483,11 @@ function codegenReactiveScope( t.assignmentExpression( "=", name, - t.memberExpression(t.identifier("$"), t.numericLiteral(index), true) + t.memberExpression( + t.identifier(cx.synthesizeName("$")), + t.numericLiteral(index), + true + ) ) ) ); @@ -460,7 +511,7 @@ function codegenReactiveScope( testCondition = t.binaryExpression( "===", t.memberExpression( - t.identifier("$"), + t.identifier(cx.synthesizeName("$")), t.numericLiteral(firstOutputIndex), true ), @@ -1341,6 +1392,7 @@ function codegenInstructionValue( new Context( cx.env, reactiveFunction.id ?? "[[ anonymous ]]", + cx.uniqueIdentifiers, cx.temp ), reactiveFunction @@ -1543,7 +1595,12 @@ function codegenInstructionValue( pruneUnusedLValues(reactiveFunction); pruneHoistedContexts(reactiveFunction); const fn = codegenReactiveFunction( - new Context(cx.env, reactiveFunction.id ?? "[[ anonymous ]]", cx.temp), + new Context( + cx.env, + reactiveFunction.id ?? "[[ anonymous ]]", + cx.uniqueIdentifiers, + cx.temp + ), reactiveFunction ).unwrap(); if (instrValue.expr.type === "ArrowFunctionExpression") { diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/RenameVariables.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/RenameVariables.ts index 4c216db573..6f09fadbbb 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/RenameVariables.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/RenameVariables.ts @@ -16,6 +16,7 @@ import { ReactiveFunction, ReactiveScopeBlock, ReactiveValue, + ValidIdentifierName, isPromotedJsxTemporary, isPromotedTemporary, makeIdentifierName, @@ -39,10 +40,15 @@ import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors"; * For temporary values that are promoted to named variables, the starting name * is "T0" for values that appear in JSX tag position and "t0" otherwise. If this * name conflicts, the number portion increments until the name is unique (t1, t2, etc). + * + * Returns a Set of all the unique variable names in the function after renaming. */ -export function renameVariables(fn: ReactiveFunction): void { +export function renameVariables( + fn: ReactiveFunction +): Set { const scopes = new Scopes(); renameVariablesImpl(fn, new Visitor(), scopes); + return scopes.names; } function renameVariablesImpl( @@ -109,6 +115,7 @@ class Visitor extends ReactiveFunctionVisitor { class Scopes { #seen: Map = new Map(); #stack: Array> = [new Map()]; + names: Set = new Set(); visit(identifier: Identifier): void { const originalName = identifier.name; @@ -134,7 +141,7 @@ class Scopes { } else if (isPromotedJsxTemporary(originalName.value)) { name = `T${id++}`; } else { - name = `${identifier.name}$${id++}`; + name = `${originalName.value}$${id++}`; } previous = this.#lookup(name); } @@ -142,6 +149,7 @@ class Scopes { identifier.name = identifierName; this.#seen.set(identifier.id, identifierName); this.#stack.at(-1)!.set(identifierName.value, identifier.id); + this.names.add(identifierName.value); } #lookup(name: string): IdentifierId | null { diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/conflicting-dollar-sign-variable.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/conflicting-dollar-sign-variable.expect.md new file mode 100644 index 0000000000..66fac2e5df --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/conflicting-dollar-sign-variable.expect.md @@ -0,0 +1,48 @@ + +## Input + +```javascript +import { identity } from "shared-runtime"; + +function Component(props) { + const $ = identity("jQuery"); + const t0 = identity([$]); + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +import { identity } from "shared-runtime"; + +function Component(props) { + const $0 = useMemoCache(1); + let t0; + if ($0[0] === Symbol.for("react.memo_cache_sentinel")) { + const $ = identity("jQuery"); + t0 = identity([$]); + $0[0] = t0; + } else { + t0 = $0[0]; + } + const t0$0 = t0; + return t0$0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: ok) ["jQuery"] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/conflicting-dollar-sign-variable.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/conflicting-dollar-sign-variable.js new file mode 100644 index 0000000000..8e42592c58 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/conflicting-dollar-sign-variable.js @@ -0,0 +1,12 @@ +import { identity } from "shared-runtime"; + +function Component(props) { + const $ = identity("jQuery"); + const t0 = identity([$]); + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/option-enable-change-variable-codegen.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/option-enable-change-variable-codegen.expect.md index 83f1f8bf4f..2502f6769a 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/option-enable-change-variable-codegen.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/option-enable-change-variable-codegen.expect.md @@ -4,8 +4,8 @@ ```javascript // @enableChangeVariableCodegen function Component(props) { - const x = [props.a, props.b.c]; - return x; + const c_0 = [props.a, props.b.c]; + return c_0; } export const FIXTURE_ENTRYPOINT = { @@ -21,10 +21,10 @@ export const FIXTURE_ENTRYPOINT = { import { unstable_useMemoCache as useMemoCache } from "react"; // @enableChangeVariableCodegen function Component(props) { const $ = useMemoCache(3); - const c_0 = $[0] !== props.a; + const c_00 = $[0] !== props.a; const c_1 = $[1] !== props.b.c; let t0; - if (c_0 || c_1) { + if (c_00 || c_1) { t0 = [props.a, props.b.c]; $[0] = props.a; $[1] = props.b.c; @@ -32,8 +32,8 @@ function Component(props) { } else { t0 = $[2]; } - const x = t0; - return x; + const c_0 = t0; + return c_0; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/option-enable-change-variable-codegen.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/option-enable-change-variable-codegen.js index e316c1db92..292c1e3787 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/option-enable-change-variable-codegen.js +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/option-enable-change-variable-codegen.js @@ -1,7 +1,7 @@ // @enableChangeVariableCodegen function Component(props) { - const x = [props.a, props.b.c]; - return x; + const c_0 = [props.a, props.b.c]; + return c_0; } export const FIXTURE_ENTRYPOINT = {