Skip to content

Commit dc09e25

Browse files
hi-ogawahi-ogawa
authored
Merge branch 'main' into fix-use-after-lazy
2 parents 3eb39ae + dffacc7 commit dc09e25

File tree

69 files changed

+246
-272
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

69 files changed

+246
-272
lines changed

‎compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {Effect, ValueKind, ValueReason} from './HIR';
99
import {
1010
BUILTIN_SHAPES,
1111
BuiltInArrayId,
12+
BuiltInAutodepsId,
1213
BuiltInFireFunctionId,
1314
BuiltInFireId,
1415
BuiltInMapId,
@@ -780,6 +781,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
780781
BuiltInUseEffectEventId,
781782
),
782783
],
784+
['AUTODEPS', addObject(DEFAULT_SHAPES, BuiltInAutodepsId, [])],
783785
];
784786

785787
TYPED_GLOBALS.push(

‎compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,7 @@ export const BuiltInFireId = 'BuiltInFire';
384384
export const BuiltInFireFunctionId = 'BuiltInFireFunction';
385385
export const BuiltInUseEffectEventId = 'BuiltInUseEffectEvent';
386386
export const BuiltinEffectEventId = 'BuiltInEffectEventFunction';
387+
export const BuiltInAutodepsId = 'BuiltInAutoDepsId';
387388

388389
// See getReanimatedModuleType() in Globals.ts — this is part of supporting Reanimated's ref-like types
389390
export const ReanimatedSharedValueId = 'ReanimatedSharedValueId';

‎compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ import {
5757
} from '../HIR/visitors';
5858
import {empty} from '../Utils/Stack';
5959
import {getOrInsertWith} from '../Utils/utils';
60+
import {deadCodeElimination} from '../Optimization';
61+
import {BuiltInAutodepsId} from '../HIR/ObjectShape';
6062

6163
/**
6264
* Infers reactive dependencies captured by useEffect lambdas and adds them as
@@ -135,7 +137,6 @@ export function inferEffectDependencies(fn: HIRFunction): void {
135137
}
136138
} else if (value.kind === 'LoadGlobal') {
137139
loadGlobals.add(lvalue.identifier.id);
138-
139140
/*
140141
* TODO: Handle properties on default exports, like
141142
* import React from 'react';
@@ -169,8 +170,17 @@ export function inferEffectDependencies(fn: HIRFunction): void {
169170
) {
170171
const callee =
171172
value.kind === 'CallExpression' ? value.callee : value.property;
173+
174+
const autodepsArgIndex = value.args.findIndex(
175+
arg =>
176+
arg.kind === 'Identifier' &&
177+
arg.identifier.type.kind === 'Object' &&
178+
arg.identifier.type.shapeId === BuiltInAutodepsId,
179+
);
172180
if (
173-
value.args.length === autodepFnLoads.get(callee.identifier.id) &&
181+
value.args.length > 1 &&
182+
autodepsArgIndex > 0 &&
183+
autodepFnLoads.has(callee.identifier.id) &&
174184
value.args[0].kind === 'Identifier'
175185
) {
176186
// We have a useEffect call with no deps array, so we need to infer the deps
@@ -260,7 +270,10 @@ export function inferEffectDependencies(fn: HIRFunction): void {
260270
effects: null,
261271
},
262272
});
263-
value.args.push({...depsPlace, effect: Effect.Freeze});
273+
value.args[autodepsArgIndex] = {
274+
...depsPlace,
275+
effect: Effect.Freeze,
276+
};
264277
fn.env.inferredEffectLocations.add(callee.loc);
265278
} else if (loadGlobals.has(value.args[0].identifier.id)) {
266279
// Global functions have no reactive dependencies, so we can insert an empty array
@@ -275,7 +288,10 @@ export function inferEffectDependencies(fn: HIRFunction): void {
275288
effects: null,
276289
},
277290
});
278-
value.args.push({...depsPlace, effect: Effect.Freeze});
291+
value.args[autodepsArgIndex] = {
292+
...depsPlace,
293+
effect: Effect.Freeze,
294+
};
279295
fn.env.inferredEffectLocations.add(callee.loc);
280296
}
281297
} else if (
@@ -323,6 +339,7 @@ export function inferEffectDependencies(fn: HIRFunction): void {
323339
// Renumber instructions and fix scope ranges
324340
markInstructionIds(fn.body);
325341
fixScopeAndIdentifierRanges(fn.body);
342+
deadCodeElimination(fn);
326343

327344
fn.env.hasInferredEffect = true;
328345
}
@@ -408,6 +425,7 @@ function rewriteSplices(
408425
rewriteBlocks.push(currBlock);
409426

410427
let cursor = 0;
428+
411429
for (const rewrite of splices) {
412430
while (originalInstrs[cursor].id < rewrite.location) {
413431
CompilerError.invariant(
@@ -429,7 +447,7 @@ function rewriteSplices(
429447

430448
if (rewrite.kind === 'instr') {
431449
currBlock.instructions.push(rewrite.value);
432-
} else {
450+
} else if (rewrite.kind === 'block') {
433451
const {entry, blocks} = rewrite.value;
434452
const entryBlock = blocks.get(entry)!;
435453
// splice in all instructions from the entry block

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-granular-access.expect.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@
33

44
```javascript
55
// @inferEffectDependencies @panicThreshold:"none"
6-
import {useEffect} from 'react';
6+
import {useEffect, AUTODEPS} from 'react';
77
import {print} from 'shared-runtime';
88

99
function Component({foo}) {
1010
const arr = [];
1111
// Taking either arr[0].value or arr as a dependency is reasonable
1212
// as long as developers know what to expect.
13-
useEffect(() => print(arr[0].value));
13+
useEffect(() => print(arr[0].value), AUTODEPS);
1414
arr.push({value: foo});
1515
return arr;
1616
}
@@ -21,7 +21,7 @@ function Component({foo}) {
2121

2222
```javascript
2323
// @inferEffectDependencies @panicThreshold:"none"
24-
import { useEffect } from "react";
24+
import { useEffect, AUTODEPS } from "react";
2525
import { print } from "shared-runtime";
2626

2727
function Component(t0) {

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-granular-access.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
// @inferEffectDependencies @panicThreshold:"none"
2-
import {useEffect} from 'react';
2+
import {useEffect, AUTODEPS} from 'react';
33
import {print} from 'shared-runtime';
44

55
function Component({foo}) {
66
const arr = [];
77
// Taking either arr[0].value or arr as a dependency is reasonable
88
// as long as developers know what to expect.
9-
useEffect(() => print(arr[0].value));
9+
useEffect(() => print(arr[0].value), AUTODEPS);
1010
arr.push({value: foo});
1111
return arr;
1212
}

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-optional-chain.expect.md‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@
33

44
```javascript
55
// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
6-
import {useEffect} from 'react';
6+
import {useEffect, AUTODEPS} from 'react';
77
import {print} from 'shared-runtime';
88

99
function Component({foo}) {
1010
const arr = [];
1111
// Taking either arr[0].value or arr as a dependency is reasonable
1212
// as long as developers know what to expect.
13-
useEffect(() => print(arr[0]?.value));
13+
useEffect(() => print(arr[0]?.value), AUTODEPS);
1414
arr.push({value: foo});
1515
return arr;
1616
}
@@ -26,7 +26,7 @@ export const FIXTURE_ENTRYPOINT = {
2626
2727
```javascript
2828
// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
29-
import { useEffect } from "react";
29+
import { useEffect, AUTODEPS } from "react";
3030
import { print } from "shared-runtime";
3131

3232
function Component(t0) {
@@ -48,9 +48,9 @@ export const FIXTURE_ENTRYPOINT = {
4848
## Logs
4949
5050
```
51-
{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":139},"end":{"line":12,"column":1,"index":384},"filename":"mutate-after-useeffect-optional-chain.ts"},"detail":{"reason":"Updating a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the mutation before calling useEffect()","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":10,"column":2,"index":345},"end":{"line":10,"column":5,"index":348},"filename":"mutate-after-useeffect-optional-chain.ts","identifierName":"arr"}}}
52-
{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":9,"column":2,"index":304},"end":{"line":9,"column":39,"index":341},"filename":"mutate-after-useeffect-optional-chain.ts"},"decorations":[{"start":{"line":9,"column":24,"index":326},"end":{"line":9,"column":27,"index":329},"filename":"mutate-after-useeffect-optional-chain.ts","identifierName":"arr"}]}
53-
{"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":139},"end":{"line":12,"column":1,"index":384},"filename":"mutate-after-useeffect-optional-chain.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}
51+
{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":149},"end":{"line":12,"column":1,"index":404},"filename":"mutate-after-useeffect-optional-chain.ts"},"detail":{"reason":"Updating a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the mutation before calling useEffect()","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":10,"column":2,"index":365},"end":{"line":10,"column":5,"index":368},"filename":"mutate-after-useeffect-optional-chain.ts","identifierName":"arr"}}}
52+
{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":9,"column":2,"index":314},"end":{"line":9,"column":49,"index":361},"filename":"mutate-after-useeffect-optional-chain.ts"},"decorations":[{"start":{"line":9,"column":24,"index":336},"end":{"line":9,"column":27,"index":339},"filename":"mutate-after-useeffect-optional-chain.ts","identifierName":"arr"}]}
53+
{"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":149},"end":{"line":12,"column":1,"index":404},"filename":"mutate-after-useeffect-optional-chain.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}
5454
```
5555
5656
### Eval output

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-optional-chain.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
2-
import {useEffect} from 'react';
2+
import {useEffect, AUTODEPS} from 'react';
33
import {print} from 'shared-runtime';
44

55
function Component({foo}) {
66
const arr = [];
77
// Taking either arr[0].value or arr as a dependency is reasonable
88
// as long as developers know what to expect.
9-
useEffect(() => print(arr[0]?.value));
9+
useEffect(() => print(arr[0]?.value), AUTODEPS);
1010
arr.push({value: foo});
1111
return arr;
1212
}

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.expect.md‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@
44
```javascript
55
// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
66

7-
import {useEffect, useRef} from 'react';
7+
import {useEffect, useRef, AUTODEPS} from 'react';
88
import {print} from 'shared-runtime';
99

1010
function Component({arrRef}) {
1111
// Avoid taking arr.current as a dependency
12-
useEffect(() => print(arrRef.current));
12+
useEffect(() => print(arrRef.current), AUTODEPS);
1313
arrRef.current.val = 2;
1414
return arrRef;
1515
}
@@ -26,7 +26,7 @@ export const FIXTURE_ENTRYPOINT = {
2626
```javascript
2727
// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
2828

29-
import { useEffect, useRef } from "react";
29+
import { useEffect, useRef, AUTODEPS } from "react";
3030
import { print } from "shared-runtime";
3131

3232
function Component(t0) {
@@ -47,9 +47,9 @@ export const FIXTURE_ENTRYPOINT = {
4747
## Logs
4848

4949
```
50-
{"kind":"CompileError","fnLoc":{"start":{"line":6,"column":0,"index":148},"end":{"line":11,"column":1,"index":311},"filename":"mutate-after-useeffect-ref-access.ts"},"detail":{"reason":"Mutating component props or hook arguments is not allowed. Consider using a local variable instead","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":9,"column":2,"index":269},"end":{"line":9,"column":16,"index":283},"filename":"mutate-after-useeffect-ref-access.ts"}}}
51-
{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":8,"column":2,"index":227},"end":{"line":8,"column":40,"index":265},"filename":"mutate-after-useeffect-ref-access.ts"},"decorations":[{"start":{"line":8,"column":24,"index":249},"end":{"line":8,"column":30,"index":255},"filename":"mutate-after-useeffect-ref-access.ts","identifierName":"arrRef"}]}
52-
{"kind":"CompileSuccess","fnLoc":{"start":{"line":6,"column":0,"index":148},"end":{"line":11,"column":1,"index":311},"filename":"mutate-after-useeffect-ref-access.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}
50+
{"kind":"CompileError","fnLoc":{"start":{"line":6,"column":0,"index":158},"end":{"line":11,"column":1,"index":331},"filename":"mutate-after-useeffect-ref-access.ts"},"detail":{"reason":"Mutating component props or hook arguments is not allowed. Consider using a local variable instead","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":9,"column":2,"index":289},"end":{"line":9,"column":16,"index":303},"filename":"mutate-after-useeffect-ref-access.ts"}}}
51+
{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":8,"column":2,"index":237},"end":{"line":8,"column":50,"index":285},"filename":"mutate-after-useeffect-ref-access.ts"},"decorations":[{"start":{"line":8,"column":24,"index":259},"end":{"line":8,"column":30,"index":265},"filename":"mutate-after-useeffect-ref-access.ts","identifierName":"arrRef"}]}
52+
{"kind":"CompileSuccess","fnLoc":{"start":{"line":6,"column":0,"index":158},"end":{"line":11,"column":1,"index":331},"filename":"mutate-after-useeffect-ref-access.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}
5353
```
5454
5555
### Eval output

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
22

3-
import {useEffect, useRef} from 'react';
3+
import {useEffect, useRef, AUTODEPS} from 'react';
44
import {print} from 'shared-runtime';
55

66
function Component({arrRef}) {
77
// Avoid taking arr.current as a dependency
8-
useEffect(() => print(arrRef.current));
8+
useEffect(() => print(arrRef.current), AUTODEPS);
99
arrRef.current.val = 2;
1010
return arrRef;
1111
}

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect.expect.md‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33

44
```javascript
55
// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
6-
import {useEffect} from 'react';
6+
import {useEffect, AUTODEPS} from 'react';
77

88
function Component({foo}) {
99
const arr = [];
1010
useEffect(() => {
1111
arr.push(foo);
12-
});
12+
}, AUTODEPS);
1313
arr.push(2);
1414
return arr;
1515
}
@@ -25,7 +25,7 @@ export const FIXTURE_ENTRYPOINT = {
2525

2626
```javascript
2727
// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
28-
import { useEffect } from "react";
28+
import { useEffect, AUTODEPS } from "react";
2929

3030
function Component(t0) {
3131
const { foo } = t0;
@@ -47,9 +47,9 @@ export const FIXTURE_ENTRYPOINT = {
4747
## Logs
4848

4949
```
50-
{"kind":"CompileError","fnLoc":{"start":{"line":4,"column":0,"index":101},"end":{"line":11,"column":1,"index":222},"filename":"mutate-after-useeffect.ts"},"detail":{"reason":"Updating a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the mutation before calling useEffect()","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":9,"column":2,"index":194},"end":{"line":9,"column":5,"index":197},"filename":"mutate-after-useeffect.ts","identifierName":"arr"}}}
51-
{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":6,"column":2,"index":149},"end":{"line":8,"column":4,"index":190},"filename":"mutate-after-useeffect.ts"},"decorations":[{"start":{"line":7,"column":4,"index":171},"end":{"line":7,"column":7,"index":174},"filename":"mutate-after-useeffect.ts","identifierName":"arr"},{"start":{"line":7,"column":4,"index":171},"end":{"line":7,"column":7,"index":174},"filename":"mutate-after-useeffect.ts","identifierName":"arr"},{"start":{"line":7,"column":13,"index":180},"end":{"line":7,"column":16,"index":183},"filename":"mutate-after-useeffect.ts","identifierName":"foo"}]}
52-
{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":101},"end":{"line":11,"column":1,"index":222},"filename":"mutate-after-useeffect.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}
50+
{"kind":"CompileError","fnLoc":{"start":{"line":4,"column":0,"index":111},"end":{"line":11,"column":1,"index":242},"filename":"mutate-after-useeffect.ts"},"detail":{"reason":"Updating a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the mutation before calling useEffect()","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":9,"column":2,"index":214},"end":{"line":9,"column":5,"index":217},"filename":"mutate-after-useeffect.ts","identifierName":"arr"}}}
51+
{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":6,"column":2,"index":159},"end":{"line":8,"column":14,"index":210},"filename":"mutate-after-useeffect.ts"},"decorations":[{"start":{"line":7,"column":4,"index":181},"end":{"line":7,"column":7,"index":184},"filename":"mutate-after-useeffect.ts","identifierName":"arr"},{"start":{"line":7,"column":4,"index":181},"end":{"line":7,"column":7,"index":184},"filename":"mutate-after-useeffect.ts","identifierName":"arr"},{"start":{"line":7,"column":13,"index":190},"end":{"line":7,"column":16,"index":193},"filename":"mutate-after-useeffect.ts","identifierName":"foo"}]}
52+
{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":111},"end":{"line":11,"column":1,"index":242},"filename":"mutate-after-useeffect.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}
5353
```
5454
5555
### Eval output

0 commit comments

Comments
 (0)