From 7666342ed32af5d3c1f4e9f053a6473530af624f Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Mon, 26 Aug 2024 12:20:28 -0700 Subject: [PATCH] [compiler] Allow inferred non-optional paths when manual deps were optional If the inferred deps are more precise (non-optional) than the manual deps (optional) it should pass validation. The other direction also seems like it would be fine - inferring optional deps when the original was non-optional - but for now let's keep the "at least as precise" rule. ghstack-source-id: 8f34860deafefe48c7a374caa1b55482e571308d Pull Request resolved: https://github.com/facebook/react/pull/30816 --- .../ValidatePreservedManualMemoization.ts | 12 +++-- ...as-memo-dep-non-optional-in-body.expect.md | 50 +++++++++++++++++++ ...ession-as-memo-dep-non-optional-in-body.js | 9 ++++ 3 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep-non-optional-in-body.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep-non-optional-in-body.js 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 6d948fad9711a..4f0c756585cf0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts @@ -167,12 +167,16 @@ function compareDeps( let isSubpath = true; for (let i = 0; i < Math.min(inferred.path.length, source.path.length); i++) { - if ( - inferred.path[i].property !== source.path[i].property || - inferred.path[i].optional !== source.path[i].optional - ) { + if (inferred.path[i].property !== source.path[i].property) { isSubpath = false; break; + } else if (inferred.path[i].optional && !source.path[i].optional) { + /** + * The inferred path must be at least as precise as the manual path: + * if the inferred path is optional, then the source path must have + * been optional too. + */ + return CompareDependencyResult.PathDifference; } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep-non-optional-in-body.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep-non-optional-in-body.expect.md new file mode 100644 index 0000000000000..7cf1bcd90b339 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep-non-optional-in-body.expect.md @@ -0,0 +1,50 @@ + +## Input + +```javascript +// @validatePreserveExistingMemoizationGuarantees +function Component(props) { + const data = useMemo(() => { + // actual code is non-optional + return props.items.edges.nodes ?? []; + // deps are optional + }, [props.items?.edges?.nodes]); + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees +function Component(props) { + const $ = _c(4); + + props.items?.edges?.nodes; + let t0; + let t1; + if ($[0] !== props.items.edges.nodes) { + t1 = props.items.edges.nodes ?? []; + $[0] = props.items.edges.nodes; + $[1] = t1; + } else { + t1 = $[1]; + } + t0 = t1; + const data = t0; + let t2; + if ($[2] !== data) { + t2 = ; + $[2] = data; + $[3] = t2; + } else { + t2 = $[3]; + } + return t2; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep-non-optional-in-body.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep-non-optional-in-body.js new file mode 100644 index 0000000000000..1a6196a494e66 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep-non-optional-in-body.js @@ -0,0 +1,9 @@ +// @validatePreserveExistingMemoizationGuarantees +function Component(props) { + const data = useMemo(() => { + // actual code is non-optional + return props.items.edges.nodes ?? []; + // deps are optional + }, [props.items?.edges?.nodes]); + return ; +}