diff --git a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidatePreservedManualMemoization.ts b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidatePreservedManualMemoization.ts index d401c51591..bebf731015 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidatePreservedManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidatePreservedManualMemoization.ts @@ -101,24 +101,59 @@ function prettyPrintScopeDependency(val: ReactiveScopeDependency): string { } return `${rootStr}${val.path.length > 0 ? "." : ""}${val.path.join(".")}`; } -function depsEqual( - dep1: ManualMemoDependency, - dep2: ManualMemoDependency + +function compareDeps( + inferred: ManualMemoDependency, + source: ManualMemoDependency ): boolean { const rootsEqual = - (dep1.root.kind === "Global" && - dep2.root.kind === "Global" && - dep1.root.identifierName === dep2.root.identifierName) || - (dep1.root.kind === "NamedLocal" && - dep2.root.kind === "NamedLocal" && - dep1.root.value.identifier.id === dep2.root.value.identifier.id); - return ( - rootsEqual && - dep1.path.length === dep2.path.length && - dep1.path.every((val, idx) => val === dep2.path[idx]) - ); + (inferred.root.kind === "Global" && + source.root.kind === "Global" && + inferred.root.identifierName === source.root.identifierName) || + (inferred.root.kind === "NamedLocal" && + source.root.kind === "NamedLocal" && + inferred.root.value.identifier.id === source.root.value.identifier.id); + if (!rootsEqual) { + return false; + } + + let isSubpath = true; + for (let i = 0; i < Math.min(inferred.path.length, source.path.length); i++) { + if (inferred.path[i] !== source.path[i]) { + isSubpath = false; + break; + } + } + + if ( + isSubpath && + (source.path.length === inferred.path.length || + (inferred.path.length >= source.path.length && + !inferred.path.includes("current"))) + ) { + return true; + } else { + return false; + } } +/** + * Validate that an inferred dependency either matches a source dependency + * or is produced by earlier instructions in the same manual memoization + * call. + * Inferred dependency `rootA.[pathA]` matches a source dependency `rootB.[pathB]` + * when: + * - rootA and rootB are loads from the same named identifier. Note that this + * identifier must be also named in source, as DropManualMemoization, which + * runs before any renaming passes, only records loads from named variables. + * - and one of the following holds: + * - pathA and pathB are identifical + * - pathB is a subpath of pathA and neither read into a `ref` type* + * + * We do not allow for partial matches on ref types because they are not immutable + * values, e.g. + * ref_prev === ref_new does not imply ref_prev.current === ref_new.current + */ function validateInferredDep( dep: ReactiveScopeDependency, temporaries: Map, @@ -154,22 +189,19 @@ function validateInferredDep( path: [...dep.path], }; } - for (const originalDep of validDepsInMemoBlock) { - if (depsEqual(originalDep, normalizedDep)) { - return; - } - } for (const decl of declsWithinMemoBlock) { - const normalizedDecl = temporaries.get(decl); - if (normalizedDecl != null && depsEqual(normalizedDecl, normalizedDep)) { - return; - } else if ( + if ( normalizedDep.root.kind === "NamedLocal" && decl === normalizedDep.root.value.identifier.id ) { return; } } + for (const originalDep of validDepsInMemoBlock) { + if (compareDeps(normalizedDep, originalDep)) { + return; + } + } errorState.push({ severity: ErrorSeverity.Todo, reason: diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useCallback-infer-more-specific.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useCallback-infer-more-specific.expect.md deleted file mode 100644 index 0168421465..0000000000 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useCallback-infer-more-specific.expect.md +++ /dev/null @@ -1,28 +0,0 @@ - -## Input - -```javascript -// @validatePreserveExistingMemoizationGuarantees - -import { useCallback } from "react"; - -// False positive as more specific memoization always results -// in fewer memo block executions. -// Precisely: -// x_new != x_prev does not imply x.y.z_new != x.y.z_prev -// x.y.z_new != x.y.z_prev does imply x_new != x_prev -// One fix would be to depend on optional chains -function useHook(x) { - return useCallback(() => [x.y.z], [x]); -} - -``` - - -## Error - -``` -[ReactForget] Todo: Could not preserve manual memoization because an inferred dependency does not match the dependency list in source. The inferred dependency was `x.y.z`, but the source dependencies were [x] -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-more-specific.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-more-specific.expect.md deleted file mode 100644 index 37ab5f5543..0000000000 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-more-specific.expect.md +++ /dev/null @@ -1,27 +0,0 @@ - -## Input - -```javascript -// @validatePreserveExistingMemoizationGuarantees - -import { useMemo } from "react"; - -// False positive as more specific memoization always results -// in fewer memo block executions. -// Precisely: -// x_new != x_prev does not imply x.y.z_new != x.y.z_prev -// x.y.z_new != x.y.z_prev does imply x_new != x_prev -function useHook(x) { - return useMemo(() => [x.y.z], [x]); -} - -``` - - -## Error - -``` -[ReactForget] Todo: Could not preserve manual memoization because an inferred dependency does not match the dependency list in source. The inferred dependency was `x.y.z`, but the source dependencies were [x] -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-more-specific.ts b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-more-specific.ts deleted file mode 100644 index 3047953ce3..0000000000 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-more-specific.ts +++ /dev/null @@ -1,12 +0,0 @@ -// @validatePreserveExistingMemoizationGuarantees - -import { useMemo } from "react"; - -// False positive as more specific memoization always results -// in fewer memo block executions. -// Precisely: -// x_new != x_prev does not imply x.y.z_new != x.y.z_prev -// x.y.z_new != x.y.z_prev does imply x_new != x_prev -function useHook(x) { - return useMemo(() => [x.y.z], [x]); -} diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-aliased-var.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-aliased-var.expect.md index ae6c9b2146..bb99846b27 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-aliased-var.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-aliased-var.expect.md @@ -20,8 +20,6 @@ function useHook(x) { ``` [ReactForget] Todo: Could not preserve manual memoization because an inferred dependency does not match the dependency list in source. The inferred dependency was `aliasedX`, but the source dependencies were [x, aliasedProp] - -[ReactForget] Todo: Could not preserve manual memoization because an inferred dependency does not match the dependency list in source. The inferred dependency was `x.y.z`, but the source dependencies were [x, aliasedProp] ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-infer-more-specific.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-infer-more-specific.expect.md new file mode 100644 index 0000000000..0d84b304a1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-infer-more-specific.expect.md @@ -0,0 +1,58 @@ + +## Input + +```javascript +// @validatePreserveExistingMemoizationGuarantees + +import { useCallback } from "react"; + +// More specific memoization always results in fewer memo block +// executions. +// Precisely: +// x_new != x_prev does NOT imply x.y.z_new != x.y.z_prev +// x.y.z_new != x.y.z_prev does imply x_new != x_prev +function useHook(x) { + return useCallback(() => [x.y.z], [x]); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useHook, + params: [{ y: { z: 2 } }], +}; + +``` + +## Code + +```javascript +// @validatePreserveExistingMemoizationGuarantees + +import { useCallback, unstable_useMemoCache as useMemoCache } from "react"; + +// More specific memoization always results in fewer memo block +// executions. +// Precisely: +// x_new != x_prev does NOT imply x.y.z_new != x.y.z_prev +// x.y.z_new != x.y.z_prev does imply x_new != x_prev +function useHook(x) { + const $ = useMemoCache(2); + let t0; + if ($[0] !== x.y.z) { + t0 = () => [x.y.z]; + $[0] = x.y.z; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useHook, + params: [{ y: { z: 2 } }], +}; + +``` + +### Eval output +(kind: ok) "[[ function params=0 ]]" \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useCallback-infer-more-specific.ts b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-infer-more-specific.ts similarity index 50% rename from compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useCallback-infer-more-specific.ts rename to compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-infer-more-specific.ts index ddb25337a3..c2fea62574 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useCallback-infer-more-specific.ts +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-infer-more-specific.ts @@ -2,12 +2,16 @@ import { useCallback } from "react"; -// False positive as more specific memoization always results -// in fewer memo block executions. +// More specific memoization always results in fewer memo block +// executions. // Precisely: -// x_new != x_prev does not imply x.y.z_new != x.y.z_prev +// x_new != x_prev does NOT imply x.y.z_new != x.y.z_prev // x.y.z_new != x.y.z_prev does imply x_new != x_prev -// One fix would be to depend on optional chains function useHook(x) { return useCallback(() => [x.y.z], [x]); } + +export const FIXTURE_ENTRYPOINT = { + fn: useHook, + params: [{ y: { z: 2 } }], +}; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-infer-more-specific.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-infer-more-specific.expect.md new file mode 100644 index 0000000000..bd3865b67e --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-infer-more-specific.expect.md @@ -0,0 +1,60 @@ + +## Input + +```javascript +// @validatePreserveExistingMemoizationGuarantees + +import { useMemo } from "react"; + +// More specific memoization always results in fewer memo block +// executions. +// Precisely: +// x_new != x_prev does NOT imply x.y.z_new != x.y.z_prev +// x.y.z_new != x.y.z_prev does imply x_new != x_prev +function useHook(x) { + return useMemo(() => [x.y.z], [x]); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useHook, + params: [{ y: { z: 2 } }], +}; + +``` + +## Code + +```javascript +// @validatePreserveExistingMemoizationGuarantees + +import { useMemo, unstable_useMemoCache as useMemoCache } from "react"; + +// More specific memoization always results in fewer memo block +// executions. +// Precisely: +// x_new != x_prev does NOT imply x.y.z_new != x.y.z_prev +// x.y.z_new != x.y.z_prev does imply x_new != x_prev +function useHook(x) { + const $ = useMemoCache(2); + let t0; + let t1; + if ($[0] !== x.y.z) { + t1 = [x.y.z]; + $[0] = x.y.z; + $[1] = t1; + } else { + t1 = $[1]; + } + t0 = t1; + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useHook, + params: [{ y: { z: 2 } }], +}; + +``` + +### Eval output +(kind: ok) [2] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-infer-more-specific.ts b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-infer-more-specific.ts new file mode 100644 index 0000000000..c4a87b7b29 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-infer-more-specific.ts @@ -0,0 +1,17 @@ +// @validatePreserveExistingMemoizationGuarantees + +import { useMemo } from "react"; + +// More specific memoization always results in fewer memo block +// executions. +// Precisely: +// x_new != x_prev does NOT imply x.y.z_new != x.y.z_prev +// x.y.z_new != x.y.z_prev does imply x_new != x_prev +function useHook(x) { + return useMemo(() => [x.y.z], [x]); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useHook, + params: [{ y: { z: 2 } }], +};