Skip to content

Commit 591c244

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 c8d53fc commit 591c244

File tree

7 files changed

+255
-1
lines changed

7 files changed

+255
-1
lines changed

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

Lines changed: 55 additions & 1 deletion
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
*/
@@ -246,6 +246,12 @@ class Context {
246246
*/
247247
#loadGlobalInstructionIds = new Map<IdentifierId, InstructionId>();
248248

249+
/*
250+
* We keep track of array expressions so we can rewrite dependency arrays passed to useEffect
251+
* to use the fire functions
252+
*/
253+
#arrayExpressions = new Map<IdentifierId, ArrayExpression>();
254+
249255
pushError(error: CompilerErrorDetailOptions): void {
250256
this.#errors.push(error);
251257
}
@@ -354,6 +360,14 @@ class Context {
354360
return this.#loadGlobalInstructionIds.get(id);
355361
}
356362

363+
addArrayExpression(id: IdentifierId, array: ArrayExpression): void {
364+
this.#arrayExpressions.set(id, array);
365+
}
366+
367+
getArrayExpression(id: IdentifierId): ArrayExpression | undefined {
368+
return this.#arrayExpressions.get(id);
369+
}
370+
357371
hasErrors(): boolean {
358372
return this.#errors.hasErrors();
359373
}
@@ -527,6 +541,44 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void {
527541
context,
528542
capturedCallees,
529543
);
544+
545+
if (
546+
value.args.length > 1 &&
547+
value.args[1] != null &&
548+
value.args[1].kind === 'Identifier'
549+
) {
550+
const depArray = value.args[1];
551+
const depArrayExpression = context.getArrayExpression(
552+
depArray.identifier.id,
553+
);
554+
if (depArrayExpression != null) {
555+
for (const dependency of depArrayExpression.elements) {
556+
if (dependency.kind === 'Identifier') {
557+
const loadOfDependency = context.getLoadLocalInstr(
558+
dependency.identifier.id,
559+
);
560+
if (loadOfDependency != null) {
561+
const replacedDepArrayItem = capturedCallees.get(
562+
loadOfDependency.place.identifier.id,
563+
);
564+
if (replacedDepArrayItem != null) {
565+
loadOfDependency.place =
566+
replacedDepArrayItem.fireFunctionBinding;
567+
}
568+
}
569+
}
570+
}
571+
} else {
572+
context.pushError({
573+
loc: value.loc,
574+
description:
575+
'You must use an array literal for an effect dependency array when that effect uses `fire()`',
576+
severity: ErrorSeverity.Invariant,
577+
reason: CANNOT_COMPILE_FIRE,
578+
suggestions: null,
579+
});
580+
}
581+
}
530582
}
531583
} else if (
532584
value.kind === 'CallExpression' &&
@@ -628,6 +680,8 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void {
628680
deleteInstrs.add(instr.id);
629681
} else if (value.kind === 'LoadGlobal') {
630682
context.addLoadGlobalInstrId(lvalue.identifier.id, instr.id);
683+
} else if (value.kind === 'ArrayExpression') {
684+
context.addArrayExpression(lvalue.identifier.id, value);
631685
}
632686
}
633687
block.instructions = rewriteInstructions(rewriteInstrs, block.instructions);
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)