[compiler] Stop using function `dependencies` in propagateScopeDeps (#31200)
Recursively visit inner function instructions to extract dependencies instead of using `LoweredFunction.dependencies` directly. This is currently gated by enableFunctionDependencyRewrite, which needs to be removed before we delete `LoweredFunction.dependencies` altogether (#31204). Some nice side effects - optional-chaining deps for inner functions - full DCE and outlining for inner functions (see #31202) - fewer extraneous instructions (see #31204) - --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31200). * #31202 * #31203 * #31201 * __->__ #31200 * #31521
mofeiZ committed
Nov 15, 2024 at 13:06 UTC
c09402aa2fc4da56f6ecabe5f5a042436b277a57
12 files changed
+159
-170
compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts
+2
@@ -231,6 +231,8 @@ const EnvironmentConfigSchema = z.object({
231
*/
232
enableUseTypeAnnotations: z.boolean().default(false),
233
234
+ enableFunctionDependencyRewrite: z.boolean().default(true),
235
+
236
/**
237
* Enables inlining ReactElement object literals in place of JSX
238
* An alternative to the standard JSX transform which replaces JSX with React's jsxProd() runtime
compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts
+43
-24
@@ -663,35 +663,54 @@ function collectDependencies(
663
664
const scopeTraversal = new ScopeBlockTraversal();
665
666
- for (const [blockId, block] of fn.body.blocks) {
667
- scopeTraversal.recordScopes(block);
668
- const scopeBlockInfo = scopeTraversal.blockInfos.get(blockId);
669
- if (scopeBlockInfo?.kind === 'begin') {
670
- context.enterScope(scopeBlockInfo.scope);
671
- } else if (scopeBlockInfo?.kind === 'end') {
672
- context.exitScope(scopeBlockInfo.scope, scopeBlockInfo?.pruned);
673
- }
674
-
675
- // Record referenced optional chains in phis
676
- for (const phi of block.phis) {
677
- for (const operand of phi.operands) {
678
- const maybeOptionalChain = temporaries.get(operand[1].identifier.id);
679
- if (maybeOptionalChain) {
680
- context.visitDependency(maybeOptionalChain);
666
+ const handleFunction = (fn: HIRFunction): void => {
667
+ for (const [blockId, block] of fn.body.blocks) {
668
+ scopeTraversal.recordScopes(block);
669
+ const scopeBlockInfo = scopeTraversal.blockInfos.get(blockId);
670
+ if (scopeBlockInfo?.kind === 'begin') {
671
+ context.enterScope(scopeBlockInfo.scope);
672
+ } else if (scopeBlockInfo?.kind === 'end') {
673
+ context.exitScope(scopeBlockInfo.scope, scopeBlockInfo.pruned);
674
+ }
675
+ // Record referenced optional chains in phis
676
+ for (const phi of block.phis) {
677
+ for (const operand of phi.operands) {
678
+ const maybeOptionalChain = temporaries.get(operand[1].identifier.id);
679
+ if (maybeOptionalChain) {
680
+ context.visitDependency(maybeOptionalChain);
681
+ }
682
}
683
}
683
- }
684
- for (const instr of block.instructions) {
685
- if (!processedInstrsInOptional.has(instr)) {
686
- handleInstruction(instr, context);
684
+ for (const instr of block.instructions) {
685
+ if (
686
+ fn.env.config.enableFunctionDependencyRewrite &&
687
+ (instr.value.kind === 'FunctionExpression' ||
688
+ instr.value.kind === 'ObjectMethod')
689
+ ) {
690
+ context.declare(instr.lvalue.identifier, {
691
+ id: instr.id,
692
+ scope: context.currentScope,
693
+ });
694
+ /**
695
+ * Recursively visit the inner function to extract dependencies there
696
+ */
697
+ const wasInInnerFn = context.inInnerFn;
698
+ context.inInnerFn = true;
699
+ handleFunction(instr.value.loweredFunc.func);
700
+ context.inInnerFn = wasInInnerFn;
701
+ } else if (!processedInstrsInOptional.has(instr)) {
702
+ handleInstruction(instr, context);
703
+ }
704
}
688
- }
705
690
- if (!processedInstrsInOptional.has(block.terminal)) {
691
- for (const place of eachTerminalOperand(block.terminal)) {
692
- context.visitOperand(place);
706
+ if (!processedInstrsInOptional.has(block.terminal)) {
707
+ for (const place of eachTerminalOperand(block.terminal)) {
708
+ context.visitOperand(place);
709
+ }
710
}
711
}
695
- }
712
+ };
713
+
714
+ handleFunction(fn);
715
return context.deps;
716
}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate-2.expect.md
+6
-15
@@ -26,29 +26,20 @@ export const FIXTURE_ENTRYPOINT = {
26
```javascript
27
import { c as _c } from "react/compiler-runtime";
28
function component(a, b) {
29
- const $ = _c(5);
30
- let t0;
31
- if ($[0] !== b) {
32
- t0 = { b };
33
- $[0] = b;
34
- $[1] = t0;
35
- } else {
36
- t0 = $[1];
37
- }
38
- const y = t0;
29
+ const $ = _c(2);
30
+ const y = { b };
31
let z;
40
- if ($[2] !== a || $[3] !== y) {
32
+ if ($[0] !== a) {
33
z = { a };
34
const x = function () {
35
z.a = 2;
36
};
37
38
x();
47
- $[2] = a;
48
- $[3] = y;
49
- $[4] = z;
39
+ $[0] = a;
40
+ $[1] = z;
41
} else {
51
- z = $[4];
42
+ z = $[1];
43
}
44
return z;
45
}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md
new
+53
@@ -0,0 +1,53 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @validatePreserveExistingMemoizationGuarantees
6
+import {useCallback} from 'react';
7
+import {Stringify} from 'shared-runtime';
8
+
9
+/**
10
+ * TODO: we're currently bailing out because `contextVar` is a context variable
11
+ * and not recorded into the PropagateScopeDeps LoadLocal / PropertyLoad
12
+ * sidemap. Previously, we were able to avoid this as `BuildHIR` hoisted
13
+ * `LoadContext` and `PropertyLoad` instructions into the outer function, which
14
+ * we took as eligible dependencies.
15
+ *
16
+ * One solution is to simply record `LoadContext` identifiers into the
17
+ * temporaries sidemap when the instruction occurs *after* the context
18
+ * variable's mutable range.
19
+ */
20
+function Foo(props) {
21
+ let contextVar;
22
+ if (props.cond) {
23
+ contextVar = {val: 2};
24
+ } else {
25
+ contextVar = {};
26
+ }
27
+
28
+ const cb = useCallback(() => [contextVar.val], [contextVar.val]);
29
+
30
+ return <Stringify cb={cb} shouldInvokeFns={true} />;
31
+}
32
+
33
+export const FIXTURE_ENTRYPOINT = {
34
+ fn: Foo,
35
+ params: [{cond: true}],
36
+};
37
+
38
+```
39
+
40
+
41
+## Error
42
+
43
+```
44
+ 22 | }
45
+ 23 |
46
+> 24 | const cb = useCallback(() => [contextVar.val], [contextVar.val]);
47
+ | ^^^^^^^^^^^^^^^^^^^^^^ 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 (24:24)
48
+ 25 |
49
+ 26 | return <Stringify cb={cb} shouldInvokeFns={true} />;
50
+ 27 | }
51
+```
52
+
53
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.tsx
new
+32
@@ -0,0 +1,32 @@
1
+// @validatePreserveExistingMemoizationGuarantees
2
+import {useCallback} from 'react';
3
+import {Stringify} from 'shared-runtime';
4
+
5
+/**
6
+ * TODO: we're currently bailing out because `contextVar` is a context variable
7
+ * and not recorded into the PropagateScopeDeps LoadLocal / PropertyLoad
8
+ * sidemap. Previously, we were able to avoid this as `BuildHIR` hoisted
9
+ * `LoadContext` and `PropertyLoad` instructions into the outer function, which
10
+ * we took as eligible dependencies.
11
+ *
12
+ * One solution is to simply record `LoadContext` identifiers into the
13
+ * temporaries sidemap when the instruction occurs *after* the context
14
+ * variable's mutable range.
15
+ */
16
+function Foo(props) {
17
+ let contextVar;
18
+ if (props.cond) {
19
+ contextVar = {val: 2};
20
+ } else {
21
+ contextVar = {};
22
+ }
23
+
24
+ const cb = useCallback(() => [contextVar.val], [contextVar.val]);
25
+
26
+ return <Stringify cb={cb} shouldInvokeFns={true} />;
27
+}
28
+
29
+export const FIXTURE_ENTRYPOINT = {
30
+ fn: Foo,
31
+ params: [{cond: true}],
32
+};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-infer-less-specific-conditional-access.expect.md
-2
@@ -44,8 +44,6 @@ function Component({propA, propB}) {
44
| ^^^^^^^^^^^^^^^^^
45
> 14 | }, [propA?.a, propB.x.y]);
46
| ^^^^ 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)
47
-
48
-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)
47
15 | }
48
16 |
49
```
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md
deleted
-81
@@ -1,81 +0,0 @@
1
-
2
-## Input
3
-
4
-```javascript
5
-// @validatePreserveExistingMemoizationGuarantees
6
-import {useCallback} from 'react';
7
-import {Stringify} from 'shared-runtime';
8
-
9
-function Foo(props) {
10
- let contextVar;
11
- if (props.cond) {
12
- contextVar = {val: 2};
13
- } else {
14
- contextVar = {};
15
- }
16
-
17
- const cb = useCallback(() => [contextVar.val], [contextVar.val]);
18
-
19
- return <Stringify cb={cb} shouldInvokeFns={true} />;
20
-}
21
-
22
-export const FIXTURE_ENTRYPOINT = {
23
- fn: Foo,
24
- params: [{cond: true}],
25
-};
26
-
27
-```
28
-
29
-## Code
30
-
31
-```javascript
32
-import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees
33
-import { useCallback } from "react";
34
-import { Stringify } from "shared-runtime";
35
-
36
-function Foo(props) {
37
- const $ = _c(6);
38
- let contextVar;
39
- if ($[0] !== props.cond) {
40
- if (props.cond) {
41
- contextVar = { val: 2 };
42
- } else {
43
- contextVar = {};
44
- }
45
- $[0] = props.cond;
46
- $[1] = contextVar;
47
- } else {
48
- contextVar = $[1];
49
- }
50
-
51
- const t0 = contextVar;
52
- let t1;
53
- if ($[2] !== t0.val) {
54
- t1 = () => [contextVar.val];
55
- $[2] = t0.val;
56
- $[3] = t1;
57
- } else {
58
- t1 = $[3];
59
- }
60
- contextVar;
61
- const cb = t1;
62
- let t2;
63
- if ($[4] !== cb) {
64
- t2 = <Stringify cb={cb} shouldInvokeFns={true} />;
65
- $[4] = cb;
66
- $[5] = t2;
67
- } else {
68
- t2 = $[5];
69
- }
70
- return t2;
71
-}
72
-
73
-export const FIXTURE_ENTRYPOINT = {
74
- fn: Foo,
75
- params: [{ cond: true }],
76
-};
77
-
78
-```
79
-
80
-### Eval output
81
-(kind: ok) <div>{"cb":{"kind":"Function","result":[2]},"shouldInvokeFns":true}</div>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.tsx
deleted
-21
@@ -1,21 +0,0 @@
1
-// @validatePreserveExistingMemoizationGuarantees
2
-import {useCallback} from 'react';
3
-import {Stringify} from 'shared-runtime';
4
-
5
-function Foo(props) {
6
- let contextVar;
7
- if (props.cond) {
8
- contextVar = {val: 2};
9
- } else {
10
- contextVar = {};
11
- }
12
-
13
- const cb = useCallback(() => [contextVar.val], [contextVar.val]);
14
-
15
- return <Stringify cb={cb} shouldInvokeFns={true} />;
16
-}
17
-
18
-export const FIXTURE_ENTRYPOINT = {
19
- fn: Foo,
20
- params: [{cond: true}],
21
-};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context.expect.md
+7
-9
@@ -45,18 +45,16 @@ function Foo(props) {
45
} else {
46
x = $[1];
47
}
48
-
49
- const t0 = x;
50
- let t1;
51
- if ($[2] !== t0) {
52
- t1 = () => [x];
53
- $[2] = t0;
54
- $[3] = t1;
48
+ let t0;
49
+ if ($[2] !== x) {
50
+ t0 = () => [x];
51
+ $[2] = x;
52
+ $[3] = t0;
53
} else {
56
- t1 = $[3];
54
+ t0 = $[3];
55
}
56
x;
59
- const cb = t1;
57
+ const cb = t0;
58
return cb;
59
}
60
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-extended-contextvar-scope.expect.md
+12
-14
@@ -70,28 +70,26 @@ function useBar(t0, cond) {
70
if (cond) {
71
x = b;
72
}
73
-
74
- const t2 = x;
75
- let t3;
76
- if ($[1] !== a || $[2] !== t2) {
77
- t3 = () => [a, x];
73
+ let t2;
74
+ if ($[1] !== a || $[2] !== x) {
75
+ t2 = () => [a, x];
76
$[1] = a;
79
- $[2] = t2;
80
- $[3] = t3;
77
+ $[2] = x;
78
+ $[3] = t2;
79
} else {
82
- t3 = $[3];
80
+ t2 = $[3];
81
}
82
x;
85
- const cb = t3;
86
- let t4;
83
+ const cb = t2;
84
+ let t3;
85
if ($[4] !== cb) {
88
- t4 = <Stringify cb={cb} shouldInvoke={true} />;
86
+ t3 = <Stringify cb={cb} shouldInvoke={true} />;
87
$[4] = cb;
90
- $[5] = t4;
88
+ $[5] = t3;
89
} else {
92
- t4 = $[5];
90
+ t3 = $[5];
91
}
94
- return t4;
92
+ return t3;
93
}
94
95
export const FIXTURE_ENTRYPOINT = {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md
+2
-2
@@ -34,9 +34,9 @@ function useFoo(t0) {
34
const $ = _c(2);
35
const { a } = t0;
36
let t1;
37
- if ($[0] !== a.b) {
37
+ if ($[0] !== a.b?.c.d?.e) {
38
t1 = <Stringify fn={() => a.b?.c.d?.e} shouldInvokeFns={true} />;
39
- $[0] = a.b;
39
+ $[0] = a.b?.c.d?.e;
40
$[1] = t1;
41
} else {
42
t1 = $[1];
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md
+2
-2
@@ -31,9 +31,9 @@ function useFoo(t0) {
31
const $ = _c(2);
32
const { a } = t0;
33
let t1;
34
- if ($[0] !== a.b) {
34
+ if ($[0] !== a.b?.c.d?.e) {
35
t1 = <Stringify fn={() => a.b?.c.d?.e} shouldInvokeFns={true} />;
36
- $[0] = a.b;
36
+ $[0] = a.b?.c.d?.e;
37
$[1] = t1;
38
} else {
39
t1 = $[1];