Skip to content

Commit 2bcbf25

Browse files
authored
[compiler] Fix false positive for useMemo reassigning context vars (#34904)
Within a function expression local variables may use StoreContext for local context variables, so the reassignment check here was firing too often. We should only report an error for variables that are declared outside the function, ie part of its `context`. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34904). * #34903 * __->__ #34904
1 parent aaad0ea commit 2bcbf25

File tree

3 files changed

+74
-14
lines changed

3 files changed

+74
-14
lines changed

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -184,25 +184,28 @@ function validateNoContextVariableAssignment(
184184
fn: HIRFunction,
185185
errors: CompilerError,
186186
): void {
187+
const context = new Set(fn.context.map(place => place.identifier.id));
187188
for (const block of fn.body.blocks.values()) {
188189
for (const instr of block.instructions) {
189190
const value = instr.value;
190191
switch (value.kind) {
191192
case 'StoreContext': {
192-
errors.pushDiagnostic(
193-
CompilerDiagnostic.create({
194-
category: ErrorCategory.UseMemo,
195-
reason:
196-
'useMemo() callbacks may not reassign variables declared outside of the callback',
197-
description:
198-
'useMemo() callbacks must be pure functions and cannot reassign variables defined outside of the callback function',
199-
suggestions: null,
200-
}).withDetails({
201-
kind: 'error',
202-
loc: value.lvalue.place.loc,
203-
message: 'Cannot reassign variable',
204-
}),
205-
);
193+
if (context.has(value.lvalue.place.identifier.id)) {
194+
errors.pushDiagnostic(
195+
CompilerDiagnostic.create({
196+
category: ErrorCategory.UseMemo,
197+
reason:
198+
'useMemo() callbacks may not reassign variables declared outside of the callback',
199+
description:
200+
'useMemo() callbacks must be pure functions and cannot reassign variables defined outside of the callback function',
201+
suggestions: null,
202+
}).withDetails({
203+
kind: 'error',
204+
loc: value.lvalue.place.loc,
205+
message: 'Cannot reassign variable',
206+
}),
207+
);
208+
}
206209
break;
207210
}
208211
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @flow
6+
export hook useItemLanguage(items) {
7+
return useMemo(() => {
8+
let language: ?string = null;
9+
items.forEach(item => {
10+
if (item.language != null) {
11+
language = item.language;
12+
}
13+
});
14+
return language;
15+
}, [items]);
16+
}
17+
18+
```
19+
20+
## Code
21+
22+
```javascript
23+
import { c as _c } from "react/compiler-runtime";
24+
export function useItemLanguage(items) {
25+
const $ = _c(2);
26+
let language;
27+
if ($[0] !== items) {
28+
language = null;
29+
items.forEach((item) => {
30+
if (item.language != null) {
31+
language = item.language;
32+
}
33+
});
34+
$[0] = items;
35+
$[1] = language;
36+
} else {
37+
language = $[1];
38+
}
39+
return language;
40+
}
41+
42+
```
43+
44+
### Eval output
45+
(kind: exception) Fixture not implemented
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// @flow
2+
export hook useItemLanguage(items) {
3+
return useMemo(() => {
4+
let language: ?string = null;
5+
items.forEach(item => {
6+
if (item.language != null) {
7+
language = item.language;
8+
}
9+
});
10+
return language;
11+
}, [items]);
12+
}

0 commit comments

Comments
 (0)