From fcc2182641b628cd6fcf46d2ea59c2cc05f7ddb8 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 20 Dec 2023 13:52:42 -0800 Subject: [PATCH] Handle scopes with only early return and no decls/deps/reassigns Fixes the case from the previous PR by using a different sentinel for uninitialized cache values and early returns. I confirmed with console.log that the reactive scope for `x` only evaluates on the first execution, after which we figure out that we don't need to execute it again. --- .../ReactiveScopes/CodegenReactiveFunction.ts | 7 +++-- .../ReactiveScopes/PropagateEarlyReturns.ts | 3 +- .../conditional-early-return.expect.md | 12 ++++---- ...rly-return-within-reactive-scope.expect.md | 4 +-- ...tions-reassignments-dependencies.expect.md | 28 ++++++++----------- ...declarations-reassignments-dependencies.js | 6 ++-- ...rly-return-within-reactive-scope.expect.md | 4 +-- ...rly-return-within-reactive-scope.expect.md | 4 +-- ...atch-try-value-modified-in-catch.expect.md | 4 +-- .../try-catch-with-catch-param.expect.md | 4 +-- .../compiler/try-catch-with-return.expect.md | 4 +-- 11 files changed, 39 insertions(+), 41 deletions(-) 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 8453c0b502..748cbbaaab 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -42,6 +42,9 @@ import { buildReactiveFunction } from "./BuildReactiveFunction"; import { SINGLE_CHILD_FBT_TAGS } from "./MemoizeFbtOperandsInSameScope"; import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors"; +export const MEMO_CACHE_SENTINEL = "react.memo_cache_sentinel"; +export const EARLY_RETURN_SENTINEL = "react.early_return_sentinel"; + export type CodegenFunction = { type: "CodegenFunction"; id: t.Identifier | null; @@ -443,7 +446,7 @@ function codegenReactiveScope( ), t.callExpression( t.memberExpression(t.identifier("Symbol"), t.identifier("for")), - [t.stringLiteral("react.memo_cache_sentinel")] + [t.stringLiteral(MEMO_CACHE_SENTINEL)] ) ); } @@ -516,7 +519,7 @@ function codegenReactiveScope( t.identifier(scope.earlyReturnValue.value.name!), t.callExpression( t.memberExpression(t.identifier("Symbol"), t.identifier("for")), - [t.stringLiteral("react.memo_cache_sentinel")] + [t.stringLiteral(EARLY_RETURN_SENTINEL)] ) ), t.blockStatement([ diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PropagateEarlyReturns.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PropagateEarlyReturns.ts index 0b06fd3ae7..8412709dab 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PropagateEarlyReturns.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PropagateEarlyReturns.ts @@ -19,6 +19,7 @@ import { makeType, } from "../HIR"; import { createTemporaryPlace } from "../HIR/HIRBuilder"; +import { EARLY_RETURN_SENTINEL } from "./CodegenReactiveFunction"; import { ReactiveFunctionTransform, Transformed } from "./visitors"; /** @@ -191,7 +192,7 @@ class Transform extends ReactiveFunctionTransform { lvalue: { ...argTemp }, value: { kind: "Primitive", - value: "react.memo_cache_sentinel", + value: EARLY_RETURN_SENTINEL, loc, }, }, diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/conditional-early-return.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/conditional-early-return.expect.md index f525231066..6a8f8f4bdf 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/conditional-early-return.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/conditional-early-return.expect.md @@ -78,7 +78,7 @@ function ComponentA(props) { let a_DEBUG; let t37; if ($[0] !== props.a || $[1] !== props.b || $[2] !== props.d) { - t37 = Symbol.for("react.memo_cache_sentinel"); + t37 = Symbol.for("react.early_return_sentinel"); bb7: { a_DEBUG = []; a_DEBUG.push(props.a); @@ -98,7 +98,7 @@ function ComponentA(props) { a_DEBUG = $[3]; t37 = $[4]; } - if (t37 !== Symbol.for("react.memo_cache_sentinel")) { + if (t37 !== Symbol.for("react.early_return_sentinel")) { return t37; } return a_DEBUG; @@ -134,7 +134,7 @@ function ComponentC(props) { let a; let t47; if ($[0] !== props) { - t47 = Symbol.for("react.memo_cache_sentinel"); + t47 = Symbol.for("react.early_return_sentinel"); bb7: { a = []; a.push(props.a); @@ -153,7 +153,7 @@ function ComponentC(props) { a = $[1]; t47 = $[2]; } - if (t47 !== Symbol.for("react.memo_cache_sentinel")) { + if (t47 !== Symbol.for("react.early_return_sentinel")) { return t47; } return a; @@ -167,7 +167,7 @@ function ComponentD(props) { let a; let t47; if ($[0] !== props) { - t47 = Symbol.for("react.memo_cache_sentinel"); + t47 = Symbol.for("react.early_return_sentinel"); bb7: { a = []; a.push(props.a); @@ -186,7 +186,7 @@ function ComponentD(props) { a = $[1]; t47 = $[2]; } - if (t47 !== Symbol.for("react.memo_cache_sentinel")) { + if (t47 !== Symbol.for("react.early_return_sentinel")) { return t47; } return a; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-nested-early-return-within-reactive-scope.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-nested-early-return-within-reactive-scope.expect.md index c1ad413f9a..9e4990bcea 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-nested-early-return-within-reactive-scope.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-nested-early-return-within-reactive-scope.expect.md @@ -35,7 +35,7 @@ function Component(props) { const $ = useMemoCache(5); let t53; if ($[0] !== props) { - t53 = Symbol.for("react.memo_cache_sentinel"); + t53 = Symbol.for("react.early_return_sentinel"); bb11: { const x = []; if (props.cond) { @@ -74,7 +74,7 @@ function Component(props) { } else { t53 = $[1]; } - if (t53 !== Symbol.for("react.memo_cache_sentinel")) { + if (t53 !== Symbol.for("react.early_return_sentinel")) { return t53; } } diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-no-declarations-reassignments-dependencies.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-no-declarations-reassignments-dependencies.expect.md index 30c91769ab..a607f36850 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-no-declarations-reassignments-dependencies.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-no-declarations-reassignments-dependencies.expect.md @@ -15,17 +15,15 @@ import { makeArray } from "shared-runtime"; * * We have to use a distinct sentinel for the early return value. * - * Here the fixture will always take the "else" branch and never early return, and we should see that - * "recreate x" is only logged once, the first time we execute. + * Here the fixture will always take the "else" branch and never early return. Logging (not included) + * confirms that the scope for `x` only executes once, on the first render of the component. */ let ENABLE_FEATURE = false; function Component(props) { let x = []; - console.log("recreate x"); if (ENABLE_FEATURE) { x.push(42); - console.log("early return"); return x; } else { console.log("fallthrough"); @@ -66,32 +64,30 @@ import { makeArray } from "shared-runtime"; * * We have to use a distinct sentinel for the early return value. * - * Here the fixture will always take the "else" branch and never early return, and we should see that - * "recreate x" is only logged once, the first time we execute. + * Here the fixture will always take the "else" branch and never early return. Logging (not included) + * confirms that the scope for `x` only executes once, on the first render of the component. */ let ENABLE_FEATURE = false; function Component(props) { const $ = useMemoCache(3); - let t53; + let t37; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t53 = Symbol.for("react.memo_cache_sentinel"); + t37 = Symbol.for("react.early_return_sentinel"); bb8: { const x = []; - console.log("recreate x"); if (ENABLE_FEATURE) { x.push(42); - console.log("early return"); - t53 = x; + t37 = x; break bb8; } } - $[0] = t53; + $[0] = t37; } else { - t53 = $[0]; + t37 = $[0]; } - if (t53 !== Symbol.for("react.memo_cache_sentinel")) { - return t53; + if (t37 !== Symbol.for("react.early_return_sentinel")) { + return t37; } console.log("fallthrough"); @@ -132,4 +128,4 @@ export const FIXTURE_ENTRYPOINT = { [3.14] [42] [3.14] -logs: ['recreate x','fallthrough','recreate x','fallthrough','recreate x','fallthrough','recreate x','fallthrough','recreate x','fallthrough','recreate x','fallthrough','recreate x','fallthrough','recreate x','fallthrough'] \ No newline at end of file +logs: ['fallthrough','fallthrough','fallthrough','fallthrough','fallthrough','fallthrough','fallthrough','fallthrough'] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-no-declarations-reassignments-dependencies.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-no-declarations-reassignments-dependencies.js index 0b73e99714..891dc27e7b 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-no-declarations-reassignments-dependencies.js +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-no-declarations-reassignments-dependencies.js @@ -11,17 +11,15 @@ import { makeArray } from "shared-runtime"; * * We have to use a distinct sentinel for the early return value. * - * Here the fixture will always take the "else" branch and never early return, and we should see that - * "recreate x" is only logged once, the first time we execute. + * Here the fixture will always take the "else" branch and never early return. Logging (not included) + * confirms that the scope for `x` only executes once, on the first render of the component. */ let ENABLE_FEATURE = false; function Component(props) { let x = []; - console.log("recreate x"); if (ENABLE_FEATURE) { x.push(42); - console.log("early return"); return x; } else { console.log("fallthrough"); diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-within-reactive-scope.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-within-reactive-scope.expect.md index 932aff4acd..e0ac96cab5 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-within-reactive-scope.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-within-reactive-scope.expect.md @@ -49,7 +49,7 @@ function Component(props) { const $ = useMemoCache(4); let t33; if ($[0] !== props) { - t33 = Symbol.for("react.memo_cache_sentinel"); + t33 = Symbol.for("react.early_return_sentinel"); bb8: { const x = []; if (props.cond) { @@ -74,7 +74,7 @@ function Component(props) { } else { t33 = $[1]; } - if (t33 !== Symbol.for("react.memo_cache_sentinel")) { + if (t33 !== Symbol.for("react.early_return_sentinel")) { return t33; } } diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/partial-early-return-within-reactive-scope.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/partial-early-return-within-reactive-scope.expect.md index beaa0aca77..7d6898430e 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/partial-early-return-within-reactive-scope.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/partial-early-return-within-reactive-scope.expect.md @@ -35,7 +35,7 @@ function Component(props) { let y; let t46; if ($[0] !== props) { - t46 = Symbol.for("react.memo_cache_sentinel"); + t46 = Symbol.for("react.early_return_sentinel"); bb11: { const x = []; if (props.cond) { @@ -64,7 +64,7 @@ function Component(props) { y = $[1]; t46 = $[2]; } - if (t46 !== Symbol.for("react.memo_cache_sentinel")) { + if (t46 !== Symbol.for("react.early_return_sentinel")) { return t46; } return y; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-try-value-modified-in-catch.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-try-value-modified-in-catch.expect.md index a35c26bfd5..37ff7f4cfd 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-try-value-modified-in-catch.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-try-value-modified-in-catch.expect.md @@ -34,7 +34,7 @@ function Component(props) { const $ = useMemoCache(3); let t49; if ($[0] !== props.y || $[1] !== props.e) { - t49 = Symbol.for("react.memo_cache_sentinel"); + t49 = Symbol.for("react.early_return_sentinel"); bb18: { try { const y = []; @@ -56,7 +56,7 @@ function Component(props) { } else { t49 = $[2]; } - if (t49 !== Symbol.for("react.memo_cache_sentinel")) { + if (t49 !== Symbol.for("react.early_return_sentinel")) { return t49; } } diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-with-catch-param.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-with-catch-param.expect.md index 13143d7369..59fb64afec 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-with-catch-param.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-with-catch-param.expect.md @@ -35,7 +35,7 @@ function Component(props) { const $ = useMemoCache(1); let t36; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t36 = Symbol.for("react.memo_cache_sentinel"); + t36 = Symbol.for("react.early_return_sentinel"); bb11: { const x = []; try { @@ -54,7 +54,7 @@ function Component(props) { } else { t36 = $[0]; } - if (t36 !== Symbol.for("react.memo_cache_sentinel")) { + if (t36 !== Symbol.for("react.early_return_sentinel")) { return t36; } } diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-with-return.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-with-return.expect.md index 61f39dc6f1..07bbbf1363 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-with-return.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-with-return.expect.md @@ -37,7 +37,7 @@ function Component(props) { let x; let t43; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t43 = Symbol.for("react.memo_cache_sentinel"); + t43 = Symbol.for("react.early_return_sentinel"); bb25: { x = []; try { @@ -59,7 +59,7 @@ function Component(props) { x = $[0]; t43 = $[1]; } - if (t43 !== Symbol.for("react.memo_cache_sentinel")) { + if (t43 !== Symbol.for("react.early_return_sentinel")) { return t43; } return x;