@samitouri / QOS-React-2 / commits / 6907aa2a30

[compiler] Rewrite effect dep arrays that use fire (#31811)

If an effect uses a dep array, also rewrite the dep array to use the fire binding --

Jordan Brown committed Dec 20, 2024 at 17:16 UTC 6907aa2a309bdc47dc3504683159cb50b590eed8
9 files changed +320 -5
compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts
+68 -5
@@ -12,6 +12,7 @@ import {
12 SourceLocation,
13 } from '..';
14 import {
15 + ArrayExpression,
16 CallExpression,
17 Effect,
18 Environment,
@@ -38,7 +39,6 @@ import {printSourceLocationLine} from '../HIR/PrintHIR';
39
40 /*
41 * TODO(jmbrown):
41 - * - rewrite dep arrays
42 * - traverse object methods
43 * - method calls
44 * - React.useEffect calls
@@ -125,11 +125,58 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void {
125 }
126 rewriteInstrs.set(loadUseEffectInstrId, newInstrs);
127 }
128 - ensureNoRemainingCalleeCaptures(
129 - lambda.loweredFunc.func,
130 - context,
131 - capturedCallees,
128 + }
129 + ensureNoRemainingCalleeCaptures(
130 + lambda.loweredFunc.func,
131 + context,
132 + capturedCallees,
133 + );
134 +
135 + if (
136 + value.args.length > 1 &&
137 + value.args[1] != null &&
138 + value.args[1].kind === 'Identifier'
139 + ) {
140 + const depArray = value.args[1];
141 + const depArrayExpression = context.getArrayExpression(
142 + depArray.identifier.id,
143 );
144 + if (depArrayExpression != null) {
145 + for (const dependency of depArrayExpression.elements) {
146 + if (dependency.kind === 'Identifier') {
147 + const loadOfDependency = context.getLoadLocalInstr(
148 + dependency.identifier.id,
149 + );
150 + if (loadOfDependency != null) {
151 + const replacedDepArrayItem = capturedCallees.get(
152 + loadOfDependency.place.identifier.id,
153 + );
154 + if (replacedDepArrayItem != null) {
155 + loadOfDependency.place =
156 + replacedDepArrayItem.fireFunctionBinding;
157 + }
158 + }
159 + }
160 + }
161 + } else {
162 + context.pushError({
163 + loc: value.args[1].loc,
164 + description:
165 + 'You must use an array literal for an effect dependency array when that effect uses `fire()`',
166 + severity: ErrorSeverity.Invariant,
167 + reason: CANNOT_COMPILE_FIRE,
168 + suggestions: null,
169 + });
170 + }
171 + } else if (value.args.length > 1 && value.args[1].kind === 'Spread') {
172 + context.pushError({
173 + loc: value.args[1].place.loc,
174 + description:
175 + 'You must use an array literal for an effect dependency array when that effect uses `fire()`',
176 + severity: ErrorSeverity.Invariant,
177 + reason: CANNOT_COMPILE_FIRE,
178 + suggestions: null,
179 + });
180 }
181 }
182 } else if (
@@ -231,6 +278,8 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void {
278 deleteInstrs.add(instr.id);
279 } else if (value.kind === 'LoadGlobal') {
280 context.addLoadGlobalInstrId(lvalue.identifier.id, instr.id);
281 + } else if (value.kind === 'ArrayExpression') {
282 + context.addArrayExpression(lvalue.identifier.id, value);
283 }
284 }
285 block.instructions = rewriteInstructions(rewriteInstrs, block.instructions);
@@ -561,6 +610,12 @@ class Context {
610 this.#env = env;
611 }
612
613 + /*
614 + * We keep track of array expressions so we can rewrite dependency arrays passed to useEffect
615 + * to use the fire functions
616 + */
617 + #arrayExpressions = new Map<IdentifierId, ArrayExpression>();
618 +
619 pushError(error: CompilerErrorDetailOptions): void {
620 this.#errors.push(error);
621 }
@@ -655,6 +710,14 @@ class Context {
710 return this.#loadGlobalInstructionIds.get(id);
711 }
712
713 + addArrayExpression(id: IdentifierId, array: ArrayExpression): void {
714 + this.#arrayExpressions.set(id, array);
715 + }
716 +
717 + getArrayExpression(id: IdentifierId): ArrayExpression | undefined {
718 + return this.#arrayExpressions.get(id);
719 + }
720 +
721 hasErrors(): boolean {
722 return this.#errors.hasErrors();
723 }
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-no-array-literal.expect.md new
+37
@@ -0,0 +1,37 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @enableFire
6 +import {fire} from 'react';
7 +
8 +function Component(props) {
9 + const foo = props => {
10 + console.log(props);
11 + };
12 +
13 + const deps = [foo, props];
14 +
15 + useEffect(() => {
16 + fire(foo(props));
17 + }, deps);
18 +
19 + return null;
20 +}
21 +
22 +```
23 +
24 +
25 +## Error
26 +
27 +```
28 + 11 | useEffect(() => {
29 + 12 | fire(foo(props));
30 +> 13 | }, deps);
31 + | ^^^^ Invariant: Cannot compile `fire`. You must use an array literal for an effect dependency array when that effect uses `fire()` (13:13)
32 + 14 |
33 + 15 | return null;
34 + 16 | }
35 +```
36 +
37 +
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-no-array-literal.js new
+16
@@ -0,0 +1,16 @@
1 +// @enableFire
2 +import {fire} from 'react';
3 +
4 +function Component(props) {
5 + const foo = props => {
6 + console.log(props);
7 + };
8 +
9 + const deps = [foo, props];
10 +
11 + useEffect(() => {
12 + fire(foo(props));
13 + }, deps);
14 +
15 + return null;
16 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-spread.expect.md new
+37
@@ -0,0 +1,37 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @enableFire
6 +import {fire} from 'react';
7 +
8 +function Component(props) {
9 + const foo = props => {
10 + console.log(props);
11 + };
12 +
13 + const deps = [foo, props];
14 +
15 + useEffect(() => {
16 + fire(foo(props));
17 + }, ...deps);
18 +
19 + return null;
20 +}
21 +
22 +```
23 +
24 +
25 +## Error
26 +
27 +```
28 + 11 | useEffect(() => {
29 + 12 | fire(foo(props));
30 +> 13 | }, ...deps);
31 + | ^^^^ Invariant: Cannot compile `fire`. You must use an array literal for an effect dependency array when that effect uses `fire()` (13:13)
32 + 14 |
33 + 15 | return null;
34 + 16 | }
35 +```
36 +
37 +
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-spread.js new
+19
@@ -0,0 +1,19 @@
1 +// @enableFire
2 +import {fire} from 'react';
3 +
4 +function Component(props) {
5 + const foo = props => {
6 + console.log(props);
7 + };
8 +
9 + const deps = [foo, props];
10 +
11 + useEffect(
12 + () => {
13 + fire(foo(props));
14 + },
15 + ...deps
16 + );
17 +
18 + return null;
19 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/fire-and-autodeps.expect.md new
+60
@@ -0,0 +1,60 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @enableFire @inferEffectDependencies
6 +import {fire, useEffect} from 'react';
7 +
8 +function Component(props) {
9 + const foo = arg => {
10 + console.log(arg, props.bar);
11 + };
12 + useEffect(() => {
13 + fire(foo(props));
14 + });
15 +
16 + return null;
17 +}
18 +
19 +```
20 +
21 +## Code
22 +
23 +```javascript
24 +import { useFire } from "react/compiler-runtime";
25 +import { c as _c } from "react/compiler-runtime"; // @enableFire @inferEffectDependencies
26 +import { fire, useEffect } from "react";
27 +
28 +function Component(props) {
29 + const $ = _c(5);
30 + let t0;
31 + if ($[0] !== props.bar) {
32 + t0 = (arg) => {
33 + console.log(arg, props.bar);
34 + };
35 + $[0] = props.bar;
36 + $[1] = t0;
37 + } else {
38 + t0 = $[1];
39 + }
40 + const foo = t0;
41 + const t1 = useFire(foo);
42 + let t2;
43 + if ($[2] !== props || $[3] !== t1) {
44 + t2 = () => {
45 + t1(props);
46 + };
47 + $[2] = props;
48 + $[3] = t1;
49 + $[4] = t2;
50 + } else {
51 + t2 = $[4];
52 + }
53 + useEffect(t2, [t1, props]);
54 + return null;
55 +}
56 +
57 +```
58 +
59 +### Eval output
60 +(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/fire-and-autodeps.js new
+13
@@ -0,0 +1,13 @@
1 +// @enableFire @inferEffectDependencies
2 +import {fire, useEffect} from 'react';
3 +
4 +function Component(props) {
5 + const foo = arg => {
6 + console.log(arg, props.bar);
7 + };
8 + useEffect(() => {
9 + fire(foo(props));
10 + });
11 +
12 + return null;
13 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/rewrite-deps.expect.md new
+57
@@ -0,0 +1,57 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @enableFire
6 +import {fire} from 'react';
7 +
8 +function Component(props) {
9 + const foo = props => {
10 + console.log(props);
11 + };
12 + useEffect(() => {
13 + fire(foo(props));
14 + }, [foo, props]);
15 +
16 + return null;
17 +}
18 +
19 +```
20 +
21 +## Code
22 +
23 +```javascript
24 +import { useFire } from "react/compiler-runtime";
25 +import { c as _c } from "react/compiler-runtime"; // @enableFire
26 +import { fire } from "react";
27 +
28 +function Component(props) {
29 + const $ = _c(4);
30 + const foo = _temp;
31 + const t0 = useFire(foo);
32 + let t1;
33 + let t2;
34 + if ($[0] !== props || $[1] !== t0) {
35 + t1 = () => {
36 + t0(props);
37 + };
38 + t2 = [t0, props];
39 + $[0] = props;
40 + $[1] = t0;
41 + $[2] = t1;
42 + $[3] = t2;
43 + } else {
44 + t1 = $[2];
45 + t2 = $[3];
46 + }
47 + useEffect(t1, t2);
48 + return null;
49 +}
50 +function _temp(props_0) {
51 + console.log(props_0);
52 +}
53 +
54 +```
55 +
56 +### Eval output
57 +(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/rewrite-deps.js new
+13
@@ -0,0 +1,13 @@
1 +// @enableFire
2 +import {fire} from 'react';
3 +
4 +function Component(props) {
5 + const foo = props => {
6 + console.log(props);
7 + };
8 + useEffect(() => {
9 + fire(foo(props));
10 + }, [foo, props]);
11 +
12 + return null;
13 +}