diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 159656611fb..ba04d827c79 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -461,6 +461,11 @@ function codegenReactiveScope( ): void { const cacheStoreStatements: Array = []; const cacheLoadStatements: Array = []; + const cacheLoads: Array<{ + name: t.Identifier; + index: number; + value: t.Expression; + }> = []; const changeExpressions: Array = []; const changeExpressionComments: Array = []; const outputComments: Array = []; @@ -488,6 +493,10 @@ function codegenReactiveScope( } else { changeExpressions.push(comparison); } + /* + * Adding directly to cacheStoreStatements rather than cacheLoads, because there + * is no corresponding cacheLoadStatement for dependencies + */ cacheStoreStatements.push( t.expressionStatement( t.assignmentExpression( @@ -523,32 +532,7 @@ function codegenReactiveScope( t.variableDeclaration("let", [t.variableDeclarator(name)]) ); } - cacheStoreStatements.push( - t.expressionStatement( - t.assignmentExpression( - "=", - t.memberExpression( - t.identifier(cx.synthesizeName("$")), - t.numericLiteral(index), - true - ), - wrapCacheDep(cx, name) - ) - ) - ); - cacheLoadStatements.push( - t.expressionStatement( - t.assignmentExpression( - "=", - name, - t.memberExpression( - t.identifier(cx.synthesizeName("$")), - t.numericLiteral(index), - true - ) - ) - ) - ); + cacheLoads.push({ name, index, value: wrapCacheDep(cx, name) }); cx.declare(identifier); } for (const reassignment of scope.reassignments) { @@ -558,34 +542,9 @@ function codegenReactiveScope( } const name = convertIdentifier(reassignment); outputComments.push(name.name); - - cacheStoreStatements.push( - t.expressionStatement( - t.assignmentExpression( - "=", - t.memberExpression( - t.identifier(cx.synthesizeName("$")), - t.numericLiteral(index), - true - ), - wrapCacheDep(cx, name) - ) - ) - ); - cacheLoadStatements.push( - t.expressionStatement( - t.assignmentExpression( - "=", - name, - t.memberExpression( - t.identifier(cx.synthesizeName("$")), - t.numericLiteral(index), - true - ) - ) - ) - ); + cacheLoads.push({ name, index, value: wrapCacheDep(cx, name) }); } + let testCondition = (changeExpressions as Array).reduce( (acc: t.Expression | null, ident: t.Expression) => { if (acc == null) { @@ -632,67 +591,116 @@ function codegenReactiveScope( ); } let computationBlock = codegenBlock(cx, block); + let memoStatement; - const memoBlock = t.blockStatement(cacheLoadStatements); if ( cx.env.config.enableChangeDetectionForDebugging != null && changeExpressions.length > 0 ) { const detectionFunction = cx.env.config.enableChangeDetectionForDebugging.importSpecifierName; + const cacheLoadOldValueStatements: Array = []; const changeDetectionStatements: Array = []; - const oldVarDeclarationStatements: Array = []; - memoBlock.body.forEach((stmt) => { - if ( - stmt.type === "ExpressionStatement" && - stmt.expression.type === "AssignmentExpression" && - stmt.expression.left.type === "Identifier" - ) { - const name = stmt.expression.left.name; - const loadName = cx.synthesizeName(`old$${name}`); - oldVarDeclarationStatements.push( - t.variableDeclaration("let", [ - t.variableDeclarator(t.identifier(loadName)), + const idempotenceDetectionStatements: Array = []; + + for (const { name, index, value } of cacheLoads) { + const loadName = cx.synthesizeName(`old$${name.name}`); + const slot = t.memberExpression( + t.identifier(cx.synthesizeName("$")), + t.numericLiteral(index), + true + ); + cacheStoreStatements.push( + t.expressionStatement(t.assignmentExpression("=", slot, value)) + ); + cacheLoadOldValueStatements.push( + t.variableDeclaration("let", [ + t.variableDeclarator(t.identifier(loadName), slot), + ]) + ); + changeDetectionStatements.push( + t.expressionStatement( + t.callExpression(t.identifier(detectionFunction), [ + t.identifier(loadName), + name, + t.stringLiteral(name.name), + t.stringLiteral(cx.fnName), + t.stringLiteral("cached"), ]) - ); - stmt.expression.left = t.identifier(loadName); - changeDetectionStatements.push( - t.expressionStatement( - t.callExpression(t.identifier(detectionFunction), [ - t.identifier(loadName), - t.identifier(name), - t.stringLiteral(name), - t.stringLiteral(cx.fnName), - ]) - ) - ); - changeDetectionStatements.push( - t.expressionStatement( - t.assignmentExpression( - "=", - t.identifier(name), - t.identifier(loadName) - ) - ) - ); - } - }); + ) + ); + idempotenceDetectionStatements.push( + t.expressionStatement( + t.callExpression(t.identifier(detectionFunction), [ + slot, + name, + t.stringLiteral(name.name), + t.stringLiteral(cx.fnName), + t.stringLiteral("recomputed"), + ]) + ) + ); + idempotenceDetectionStatements.push( + t.expressionStatement(t.assignmentExpression("=", name, slot)) + ); + } + const condition = cx.synthesizeName("condition"); memoStatement = t.blockStatement([ ...computationBlock.body, + t.variableDeclaration("let", [ + t.variableDeclarator(t.identifier(condition), testCondition), + ]), t.ifStatement( - t.unaryExpression("!", testCondition), + t.unaryExpression("!", t.identifier(condition)), t.blockStatement([ - ...oldVarDeclarationStatements, - ...memoBlock.body, + ...cacheLoadOldValueStatements, ...changeDetectionStatements, ]) ), ...cacheStoreStatements, + t.ifStatement( + t.identifier(condition), + t.blockStatement([ + ...computationBlock.body, + ...idempotenceDetectionStatements, + ]) + ), ]); } else { + for (const { name, index, value } of cacheLoads) { + cacheStoreStatements.push( + t.expressionStatement( + t.assignmentExpression( + "=", + t.memberExpression( + t.identifier(cx.synthesizeName("$")), + t.numericLiteral(index), + true + ), + value + ) + ) + ); + cacheLoadStatements.push( + t.expressionStatement( + t.assignmentExpression( + "=", + name, + t.memberExpression( + t.identifier(cx.synthesizeName("$")), + t.numericLiteral(index), + true + ) + ) + ) + ); + } computationBlock.body.push(...cacheStoreStatements); - - memoStatement = t.ifStatement(testCondition, computationBlock, memoBlock); + memoStatement = t.ifStatement( + testCondition, + computationBlock, + t.blockStatement(cacheLoadStatements) + ); } if (cx.env.config.enableMemoizationComments) { @@ -734,9 +742,9 @@ function codegenReactiveScope( true ); } - if (memoBlock.body.length > 0) { + if (cacheLoadStatements.length > 0) { t.addComment( - memoBlock.body[0]!, + cacheLoadStatements[0]!, "leading", ` Inputs did not change, use cached value`, true diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/change-detect-reassign.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/change-detect-reassign.expect.md new file mode 100644 index 00000000000..19a6710f8ff --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/change-detect-reassign.expect.md @@ -0,0 +1,48 @@ + +## Input + +```javascript +// @enableChangeDetectionForDebugging +function Component(props) { + let x = null; + if (props.cond) { + x = []; + x.push(props.value); + } + return x; +} + +``` + +## Code + +```javascript +import { $structuralCheck } from "react-compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enableChangeDetectionForDebugging +function Component(props) { + const $ = _c(2); + let x = null; + if (props.cond) { + { + x = []; + x.push(props.value); + let condition = $[0] !== props.value; + if (!condition) { + let old$x = $[1]; + $structuralCheck(old$x, x, "x", "Component", "cached"); + } + $[0] = props.value; + $[1] = x; + if (condition) { + x = []; + x.push(props.value); + $structuralCheck($[1], x, "x", "Component", "recomputed"); + x = $[1]; + } + } + } + return x; +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/change-detect-reassign.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/change-detect-reassign.js new file mode 100644 index 00000000000..8ccc3d30f09 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/change-detect-reassign.js @@ -0,0 +1,9 @@ +// @enableChangeDetectionForDebugging +function Component(props) { + let x = null; + if (props.cond) { + x = []; + x.push(props.value); + } + return x; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-and-other-hook-unpruned-dependency.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-and-other-hook-unpruned-dependency.expect.md index 414c9cd143b..09506819608 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-and-other-hook-unpruned-dependency.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-and-other-hook-unpruned-dependency.expect.md @@ -43,14 +43,18 @@ function Component(props) { let t0; { t0 = f(props.x); - if (!($[0] !== props.x)) { - let old$t0; - old$t0 = $[1]; - $structuralCheck(old$t0, t0, "t0", "Component"); - t0 = old$t0; + let condition = $[0] !== props.x; + if (!condition) { + let old$t0 = $[1]; + $structuralCheck(old$t0, t0, "t0", "Component", "cached"); } $[0] = props.x; $[1] = t0; + if (condition) { + t0 = f(props.x); + $structuralCheck($[1], t0, "t0", "Component", "recomputed"); + t0 = $[1]; + } } const w = t0; const z = useOther(w); @@ -58,14 +62,18 @@ function Component(props) { let t1; { t1 =
{x}
; - if (!($[2] !== x)) { - let old$t1; - old$t1 = $[3]; - $structuralCheck(old$t1, t1, "t1", "Component"); - t1 = old$t1; + let condition = $[2] !== x; + if (!condition) { + let old$t1 = $[3]; + $structuralCheck(old$t1, t1, "t1", "Component", "cached"); } $[2] = x; $[3] = t1; + if (condition) { + t1 =
{x}
; + $structuralCheck($[3], t1, "t1", "Component", "recomputed"); + t1 = $[3]; + } } return t1; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-pruned-dependency-change-detect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-pruned-dependency-change-detect.expect.md index f44b54f99df..3b89bfbd3f8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-pruned-dependency-change-detect.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-pruned-dependency-change-detect.expect.md @@ -32,14 +32,18 @@ function Component(props) { let t1; { t1 =
{x}
; - if (!($[1] !== x)) { - let old$t1; - old$t1 = $[2]; - $structuralCheck(old$t1, t1, "t1", "Component"); - t1 = old$t1; + let condition = $[1] !== x; + if (!condition) { + let old$t1 = $[2]; + $structuralCheck(old$t1, t1, "t1", "Component", "cached"); } $[1] = x; $[2] = t1; + if (condition) { + t1 =
{x}
; + $structuralCheck($[2], t1, "t1", "Component", "recomputed"); + t1 = $[2]; + } } return t1; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-unpruned-dependency.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-unpruned-dependency.expect.md index cb399a0bc56..e99b8263150 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-unpruned-dependency.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-unpruned-dependency.expect.md @@ -39,14 +39,18 @@ function Component(props) { let t0; { t0 = f(props.x); - if (!($[0] !== props.x)) { - let old$t0; - old$t0 = $[1]; - $structuralCheck(old$t0, t0, "t0", "Component"); - t0 = old$t0; + let condition = $[0] !== props.x; + if (!condition) { + let old$t0 = $[1]; + $structuralCheck(old$t0, t0, "t0", "Component", "cached"); } $[0] = props.x; $[1] = t0; + if (condition) { + t0 = f(props.x); + $structuralCheck($[1], t0, "t0", "Component", "recomputed"); + t0 = $[1]; + } } const w = t0; const [x] = useState(w); @@ -58,15 +62,24 @@ function Component(props) { {w} ); - if (!($[2] !== x || $[3] !== w)) { - let old$t1; - old$t1 = $[4]; - $structuralCheck(old$t1, t1, "t1", "Component"); - t1 = old$t1; + let condition = $[2] !== x || $[3] !== w; + if (!condition) { + let old$t1 = $[4]; + $structuralCheck(old$t1, t1, "t1", "Component", "cached"); } $[2] = x; $[3] = w; $[4] = t1; + if (condition) { + t1 = ( +
+ {x} + {w} +
+ ); + $structuralCheck($[4], t1, "t1", "Component", "recomputed"); + t1 = $[4]; + } } return t1; } diff --git a/compiler/packages/react-compiler-runtime/src/index.ts b/compiler/packages/react-compiler-runtime/src/index.ts index aca78194ef2..f7583198113 100644 --- a/compiler/packages/react-compiler-runtime/src/index.ts +++ b/compiler/packages/react-compiler-runtime/src/index.ts @@ -258,10 +258,11 @@ export function $structuralCheck( oldValue: any, newValue: any, variableName: string, - fnName: string + fnName: string, + kind: string ): void { function error(l: string, r: string, path: string, depth: number) { - const str = `${fnName}: ${variableName}${path} changed from ${l} to ${r} at depth ${depth}`; + const str = `${fnName}: [${kind}] ${variableName}${path} changed from ${l} to ${r} at depth ${depth}`; if (seenErrors.has(str)) { return; } diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 1dcd1c2b55e..0bfa03c3971 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -498,6 +498,7 @@ const skipFilter = new Set([ "useState-pruned-dependency-change-detect", "useState-unpruned-dependency", "useState-and-other-hook-unpruned-dependency", + "change-detect-reassign", ]); export default skipFilter;