Skip to content

Commit e701651

Browse files
committed
[compiler] Rewrite effect dep arrays that use fire
If an effect uses a dep array, also rewrite the dep array to use the fire binding --
1 parent 13229fd commit e701651

File tree

7 files changed

+259
-5
lines changed

7 files changed

+259
-5
lines changed

compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
SourceLocation,
1313
} from '..';
1414
import {
15+
ArrayExpression,
1516
CallExpression,
1617
Effect,
1718
Environment,
@@ -38,7 +39,6 @@ import {printSourceLocationLine} from '../HIR/PrintHIR';
3839

3940
/*
4041
* TODO(jmbrown):
41-
* - rewrite dep arrays
4242
* - traverse object methods
4343
* - method calls
4444
* - React.useEffect calls
@@ -125,11 +125,49 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void {
125125
}
126126
rewriteInstrs.set(loadUseEffectInstrId, newInstrs);
127127
}
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,
132143
);
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.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+
}
133171
}
134172
}
135173
} else if (
@@ -231,6 +269,8 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void {
231269
deleteInstrs.add(instr.id);
232270
} else if (value.kind === 'LoadGlobal') {
233271
context.addLoadGlobalInstrId(lvalue.identifier.id, instr.id);
272+
} else if (value.kind === 'ArrayExpression') {
273+
context.addArrayExpression(lvalue.identifier.id, value);
234274
}
235275
}
236276
block.instructions = rewriteInstructions(rewriteInstrs, block.instructions);
@@ -561,6 +601,12 @@ class Context {
561601
this.#env = env;
562602
}
563603

604+
/*
605+
* We keep track of array expressions so we can rewrite dependency arrays passed to useEffect
606+
* to use the fire functions
607+
*/
608+
#arrayExpressions = new Map<IdentifierId, ArrayExpression>();
609+
564610
pushError(error: CompilerErrorDetailOptions): void {
565611
this.#errors.push(error);
566612
}
@@ -655,6 +701,14 @@ class Context {
655701
return this.#loadGlobalInstructionIds.get(id);
656702
}
657703

704+
addArrayExpression(id: IdentifierId, array: ArrayExpression): void {
705+
this.#arrayExpressions.set(id, array);
706+
}
707+
708+
getArrayExpression(id: IdentifierId): ArrayExpression | undefined {
709+
return this.#arrayExpressions.get(id);
710+
}
711+
658712
hasErrors(): boolean {
659713
return this.#errors.hasErrors();
660714
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
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+
9 | const deps = [foo, props];
29+
10 |
30+
> 11 | useEffect(() => {
31+
| ^^^^^^^^^^^^^^^^^
32+
> 12 | fire(foo(props));
33+
| ^^^^^^^^^^^^^^^^^^^^^
34+
> 13 | }, deps);
35+
| ^^^^^^^^^^^ Invariant: Cannot compile `fire`. You must use an array literal for an effect dependency array when that effect uses `fire()` (11:13)
36+
14 |
37+
15 | return null;
38+
16 | }
39+
```
40+
41+
Original file line numberDiff line numberDiff line change
@@ -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+
}
Original file line numberDiff line numberDiff line change
@@ -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";
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
Original file line numberDiff line numberDiff line change
@@ -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+
}
Original file line numberDiff line numberDiff line change
@@ -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";
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
Original file line numberDiff line numberDiff line change
@@ -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+
}

0 commit comments

Comments
 (0)