Skip to content

Commit 532a5f7

Browse files
committed
[compiler][repro] Postfix operator is incorrectly compiled
This bug was reported via our wg and appears to only affect values created as a ref. Currently, postfix operators used in a callback gets compiled to: ```js modalId.current = modalId.current + 1; // 1 const id = modalId.current; // 1 return id; ``` which is semantically incorrect. The postfix increment operator should return the value before incrementing. In other words something like this should have been compiled instead: ```js const id = modalId.current; // 0 modalId.current = modalId.current + 1; // 1 return id; ``` This bug does not trigger when the incremented value is a plain primitive, instead there is a TODO bailout.
1 parent b7e2de6 commit 532a5f7

File tree

3 files changed

+175
-0
lines changed

3 files changed

+175
-0
lines changed
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
2+
## Input
3+
4+
```javascript
5+
import {useRef, useEffect} from 'react';
6+
7+
/**
8+
* The postfix increment operator should return the value before incrementing.
9+
* ```js
10+
* const id = count.current; // 0
11+
* count.current = count.current + 1; // 1
12+
* return id;
13+
* ```
14+
* The bug is that we currently increment the value before the expression is evaluated.
15+
* This bug does not trigger when the incremented value is a plain primitive.
16+
*
17+
* Found differences in evaluator results
18+
* Non-forget (expected):
19+
* (kind: ok) {"count":{"current":0},"updateCountPostfix":"[[ function params=0 ]]","updateCountPrefix":"[[ function params=0 ]]"}
20+
* logs: ['id = 0','count = 1']
21+
* Forget:
22+
* (kind: ok) {"count":{"current":0},"updateCountPostfix":"[[ function params=0 ]]","updateCountPrefix":"[[ function params=0 ]]"}
23+
* logs: ['id = 1','count = 1']
24+
*/
25+
function useFoo() {
26+
const count = useRef(0);
27+
const updateCountPostfix = () => {
28+
const id = count.current++;
29+
return id;
30+
};
31+
const updateCountPrefix = () => {
32+
const id = ++count.current;
33+
return id;
34+
};
35+
useEffect(() => {
36+
const id = updateCountPostfix();
37+
console.log(`id = ${id}`);
38+
console.log(`count = ${count.current}`);
39+
}, []);
40+
return {count, updateCountPostfix, updateCountPrefix};
41+
}
42+
43+
export const FIXTURE_ENTRYPOINT = {
44+
fn: useFoo,
45+
params: [],
46+
};
47+
48+
```
49+
50+
## Code
51+
52+
```javascript
53+
import { c as _c } from "react/compiler-runtime";
54+
import { useRef, useEffect } from "react";
55+
56+
/**
57+
* The postfix increment operator should return the value before incrementing.
58+
* ```js
59+
* const id = count.current; // 0
60+
* count.current = count.current + 1; // 1
61+
* return id;
62+
* ```
63+
* The bug is that we currently increment the value before the expression is evaluated.
64+
* This bug does not trigger when the incremented value is a plain primitive.
65+
*
66+
* Found differences in evaluator results
67+
* Non-forget (expected):
68+
* (kind: ok) {"count":{"current":0},"updateCountPostfix":"[[ function params=0 ]]","updateCountPrefix":"[[ function params=0 ]]"}
69+
* logs: ['id = 0','count = 1']
70+
* Forget:
71+
* (kind: ok) {"count":{"current":0},"updateCountPostfix":"[[ function params=0 ]]","updateCountPrefix":"[[ function params=0 ]]"}
72+
* logs: ['id = 1','count = 1']
73+
*/
74+
function useFoo() {
75+
const $ = _c(5);
76+
const count = useRef(0);
77+
let t0;
78+
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
79+
t0 = () => {
80+
count.current = count.current + 1;
81+
const id = count.current;
82+
return id;
83+
};
84+
$[0] = t0;
85+
} else {
86+
t0 = $[0];
87+
}
88+
const updateCountPostfix = t0;
89+
let t1;
90+
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
91+
t1 = () => {
92+
const id_0 = (count.current = count.current + 1);
93+
return id_0;
94+
};
95+
$[1] = t1;
96+
} else {
97+
t1 = $[1];
98+
}
99+
const updateCountPrefix = t1;
100+
let t2;
101+
let t3;
102+
if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
103+
t2 = () => {
104+
const id_1 = updateCountPostfix();
105+
console.log(`id = ${id_1}`);
106+
console.log(`count = ${count.current}`);
107+
};
108+
t3 = [];
109+
$[2] = t2;
110+
$[3] = t3;
111+
} else {
112+
t2 = $[2];
113+
t3 = $[3];
114+
}
115+
useEffect(t2, t3);
116+
let t4;
117+
if ($[4] === Symbol.for("react.memo_cache_sentinel")) {
118+
t4 = { count, updateCountPostfix, updateCountPrefix };
119+
$[4] = t4;
120+
} else {
121+
t4 = $[4];
122+
}
123+
return t4;
124+
}
125+
126+
export const FIXTURE_ENTRYPOINT = {
127+
fn: useFoo,
128+
params: [],
129+
};
130+
131+
```
132+
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import {useRef, useEffect} from 'react';
2+
3+
/**
4+
* The postfix increment operator should return the value before incrementing.
5+
* ```js
6+
* const id = count.current; // 0
7+
* count.current = count.current + 1; // 1
8+
* return id;
9+
* ```
10+
* The bug is that we currently increment the value before the expression is evaluated.
11+
* This bug does not trigger when the incremented value is a plain primitive.
12+
*
13+
* Found differences in evaluator results
14+
* Non-forget (expected):
15+
* (kind: ok) {"count":{"current":0},"updateCountPostfix":"[[ function params=0 ]]","updateCountPrefix":"[[ function params=0 ]]"}
16+
* logs: ['id = 0','count = 1']
17+
* Forget:
18+
* (kind: ok) {"count":{"current":0},"updateCountPostfix":"[[ function params=0 ]]","updateCountPrefix":"[[ function params=0 ]]"}
19+
* logs: ['id = 1','count = 1']
20+
*/
21+
function useFoo() {
22+
const count = useRef(0);
23+
const updateCountPostfix = () => {
24+
const id = count.current++;
25+
return id;
26+
};
27+
const updateCountPrefix = () => {
28+
const id = ++count.current;
29+
return id;
30+
};
31+
useEffect(() => {
32+
const id = updateCountPostfix();
33+
console.log(`id = ${id}`);
34+
console.log(`count = ${count.current}`);
35+
}, []);
36+
return {count, updateCountPostfix, updateCountPrefix};
37+
}
38+
39+
export const FIXTURE_ENTRYPOINT = {
40+
fn: useFoo,
41+
params: [],
42+
};

compiler/packages/snap/src/SproutTodoFilter.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,7 @@ const skipFilter = new Set([
460460
'fbt/bug-fbt-plural-multiple-function-calls',
461461
'fbt/bug-fbt-plural-multiple-mixed-call-tag',
462462
'bug-invalid-phi-as-dependency',
463+
'bug-ref-prefix-postfix-operator',
463464

464465
// 'react-compiler-runtime' not yet supported
465466
'flag-enable-emit-hook-guards',

0 commit comments

Comments
 (0)