diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts index b15ef94d92e58..365e3aa88d4af 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts @@ -280,7 +280,13 @@ class Visitor extends ReactiveFunctionVisitor { scopeMapping = new Map(); temporaries: Map = new Map(); - collectMaybeMemoDependencies( + /** + * Recursively visit values and instructions to collect declarations + * and property loads. + * @returns a @{ManualMemoDependency} representing the variable + + * property reads represented by @value + */ + recordDepsInValue( value: ReactiveValue, state: VisitorState ): ManualMemoDependency | null { @@ -289,16 +295,28 @@ class Visitor extends ReactiveFunctionVisitor { for (const instr of value.instructions) { this.visitInstruction(instr, state); } - const result = this.collectMaybeMemoDependencies(value.value, state); - + const result = this.recordDepsInValue(value.value, state); return result; } case "OptionalExpression": { - return this.collectMaybeMemoDependencies(value.value, state); + return this.recordDepsInValue(value.value, state); + } + case "ReactiveFunctionValue": { + CompilerError.throwTodo({ + reason: + "Handle ReactiveFunctionValue in ValidatePreserveManualMemoization", + loc: value.loc, + }); + } + case "ConditionalExpression": { + this.recordDepsInValue(value.test, state); + this.recordDepsInValue(value.consequent, state); + this.recordDepsInValue(value.alternate, state); + return null; } - case "ReactiveFunctionValue": - case "ConditionalExpression": case "LogicalExpression": { + this.recordDepsInValue(value.left, state); + this.recordDepsInValue(value.right, state); return null; } default: { @@ -336,7 +354,7 @@ class Visitor extends ReactiveFunctionVisitor { state.manualMemoState.decls.add(lvalId); } - const maybeDep = this.collectMaybeMemoDependencies(value, state); + const maybeDep = this.recordDepsInValue(value, state); if (lvalId != null) { if (maybeDep != null) { temporaries.set(lvalId, maybeDep); @@ -400,7 +418,10 @@ class Visitor extends ReactiveFunctionVisitor { instruction: ReactiveInstruction, state: VisitorState ): void { - this.traverseInstruction(instruction, state); + /** + * We don't invoke traverseInstructions because `recordDepsInValue` + * recursively visits ReactiveValues and instructions + */ this.recordTemporaries(instruction, state); if (instruction.value.kind === "StartMemoize") { let depsFromSource: Array | null = null; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-slow-validate-preserve-memo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-slow-validate-preserve-memo.expect.md new file mode 100644 index 0000000000000..4cfd4cd247819 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-slow-validate-preserve-memo.expect.md @@ -0,0 +1,68 @@ + +## Input + +```javascript +// @validatePreserveExistingMemoizationGuarantees + +import { Builder } from "shared-runtime"; +function useTest({ isNull, data }: { isNull: boolean; data: string }) { + const result = Builder.makeBuilder(isNull, "hello world") + ?.push("1", 2) + ?.push(3, { + a: 4, + b: 5, + c: data, + }) + ?.push(6, data) + ?.push(7, "8") + ?.push("8", Builder.makeBuilder(!isNull)?.push(9).vals)?.vals; + return result; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useTest, + params: [{ isNull: false, data: "param" }], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees + +import { Builder } from "shared-runtime"; +function useTest(t0) { + const $ = _c(3); + const { isNull, data } = t0; + let t1; + if ($[0] !== isNull || $[1] !== data) { + t1 = Builder.makeBuilder(isNull, "hello world") + ?.push("1", 2) + ?.push(3, { a: 4, b: 5, c: data }) + ?.push( + 6, + + data, + ) + ?.push(7, "8") + ?.push("8", Builder.makeBuilder(!isNull)?.push(9).vals)?.vals; + $[0] = isNull; + $[1] = data; + $[2] = t1; + } else { + t1 = $[2]; + } + const result = t1; + return result; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useTest, + params: [{ isNull: false, data: "param" }], +}; + +``` + +### Eval output +(kind: ok) ["hello world","1",2,3,{"a":4,"b":5,"c":"param"},6,"param",7,"8","8",null] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-slow-validate-preserve-memo.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-slow-validate-preserve-memo.ts new file mode 100644 index 0000000000000..0fd75e4fc361e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-slow-validate-preserve-memo.ts @@ -0,0 +1,21 @@ +// @validatePreserveExistingMemoizationGuarantees + +import { Builder } from "shared-runtime"; +function useTest({ isNull, data }: { isNull: boolean; data: string }) { + const result = Builder.makeBuilder(isNull, "hello world") + ?.push("1", 2) + ?.push(3, { + a: 4, + b: 5, + c: data, + }) + ?.push(6, data) + ?.push(7, "8") + ?.push("8", Builder.makeBuilder(!isNull)?.push(9).vals)?.vals; + return result; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useTest, + params: [{ isNull: false, data: "param" }], +}; diff --git a/compiler/packages/snap/src/sprout/shared-runtime.ts b/compiler/packages/snap/src/sprout/shared-runtime.ts index 7d4687218d0e2..a67ab298ada0c 100644 --- a/compiler/packages/snap/src/sprout/shared-runtime.ts +++ b/compiler/packages/snap/src/sprout/shared-runtime.ts @@ -312,6 +312,22 @@ export function toJSON(value: any, invokeFns: boolean = false): string { return val; }); } +export class Builder { + vals: Array = []; + static makeBuilder(isNull: boolean, ...args: Array): Builder | null { + if (isNull) { + return null; + } else { + const builder = new Builder(); + builder.push(...args); + return builder; + } + } + push(...args: Array): Builder { + this.vals.push(...args); + return this; + } +} export const ObjectWithHooks = { useFoo(): number {