From 3a45ba241c028cd0af7bf17bb4c6487d0095a10f Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 28 Aug 2024 12:12:29 -0700 Subject: [PATCH] [compiler] Enable optional dependencies by default Per title. This gives us much more granular memoization when the source used optional member expressions. Note that we only infer optional deps when the source used optionals: we don't (yet) infer optional dependencies from conditionals. ghstack-source-id: 104d0b712d09498239e926e306c4623d546463b1 Pull Request resolved: https://github.com/facebook/react/pull/30838 --- .../src/HIR/Environment.ts | 2 +- ...call-with-optional-property-load.expect.md | 4 ++-- ...less-specific-conditional-access.expect.md | 2 -- .../conditional-member-expr.expect.md | 4 ++-- .../memberexpr-join-optional-chain.expect.md | 4 ++-- .../memberexpr-join-optional-chain2.expect.md | 21 +++++++++++++------ ...epro-scope-missing-mutable-range.expect.md | 4 ++-- 7 files changed, 24 insertions(+), 17 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index a13ebb6aac..2b007f9659 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -230,7 +230,7 @@ const EnvironmentConfigSchema = z.object({ * just `props`. With this flag enabled, we'll infer that full path as * the dependency. */ - enableOptionalDependencies: z.boolean().default(false), + enableOptionalDependencies: z.boolean().default(true), /* * Enable validation of hooks to partially check that the component honors the rules of hooks. diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-call-with-optional-property-load.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-call-with-optional-property-load.expect.md index 795ab61cca..d95461adf9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-call-with-optional-property-load.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-call-with-optional-property-load.expect.md @@ -15,9 +15,9 @@ import { c as _c } from "react/compiler-runtime"; function Component(props) { const $ = _c(2); let t0; - if ($[0] !== props) { + if ($[0] !== props?.items) { t0 = props?.items?.map?.(render)?.filter(Boolean) ?? []; - $[0] = props; + $[0] = props?.items; $[1] = t0; } else { t0 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-infer-less-specific-conditional-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-infer-less-specific-conditional-access.expect.md index 0cda150692..25d7bf5f7c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-infer-less-specific-conditional-access.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-infer-less-specific-conditional-access.expect.md @@ -44,8 +44,6 @@ function Component({propA, propB}) { | ^^^^^^^^^^^^^^^^^ > 14 | }, [propA?.a, propB.x.y]); | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (6:14) - -CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (6:14) 15 | } 16 | ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/conditional-member-expr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/conditional-member-expr.expect.md index 59b48898fd..2dd61732f6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/conditional-member-expr.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/conditional-member-expr.expect.md @@ -29,10 +29,10 @@ import { c as _c } from "react/compiler-runtime"; // To preserve the nullthrows function Component(props) { const $ = _c(2); let x; - if ($[0] !== props.a) { + if ($[0] !== props.a?.b) { x = []; x.push(props.a?.b); - $[0] = props.a; + $[0] = props.a?.b; $[1] = x; } else { x = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain.expect.md index 7362cd8317..a66540655a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain.expect.md @@ -44,11 +44,11 @@ import { c as _c } from "react/compiler-runtime"; // To preserve the nullthrows function Component(props) { const $ = _c(2); let x; - if ($[0] !== props.a) { + if ($[0] !== props.a.b) { x = []; x.push(props.a?.b); x.push(props.a.b.c); - $[0] = props.a; + $[0] = props.a.b; $[1] = x; } else { x = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain2.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain2.expect.md index f25ea2a31e..8d69c008c5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain2.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain2.expect.md @@ -21,16 +21,25 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(2); + const $ = _c(5); let x; - if ($[0] !== props.items) { + if ($[0] !== props.items?.length || $[1] !== props.items?.edges) { x = []; x.push(props.items?.length); - x.push(props.items?.edges?.map?.(render)?.filter?.(Boolean) ?? []); - $[0] = props.items; - $[1] = x; + let t0; + if ($[3] !== props.items?.edges) { + t0 = props.items?.edges?.map?.(render)?.filter?.(Boolean) ?? []; + $[3] = props.items?.edges; + $[4] = t0; + } else { + t0 = $[4]; + } + x.push(t0); + $[0] = props.items?.length; + $[1] = props.items?.edges; + $[2] = x; } else { - x = $[1]; + x = $[2]; } return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md index 2ce8ffbe4c..d8e59c486a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md @@ -23,14 +23,14 @@ function HomeDiscoStoreItemTileRating(props) { const $ = _c(4); const item = useFragment(); let count; - if ($[0] !== item) { + if ($[0] !== item?.aggregates) { count = 0; const aggregates = item?.aggregates || []; aggregates.forEach((aggregate) => { count = count + (aggregate.count || 0); count; }); - $[0] = item; + $[0] = item?.aggregates; $[1] = count; } else { count = $[1];