Skip to content

Commit eee5ca2

Browse files
authored
[compiler] Prune all unused array destructure items during DCE (#31619)
We didn't originally support holes within array patterns, so DCE was only able to prune unused items from the end of an array pattern. Now that we support holes we can replace any unused item with a hole, and then just prune the items to the last identifier/spread entry. Note: this was motivated by finding useState where either the state or setState go unused — both are strong indications that you're violating the rules in some way. By DCE-ing the unused portions of the useState destructuring we can easily check if you're ignoring either value. closes #31603 This is a redo of that PR not using ghstack
1 parent aba370f commit eee5ca2

12 files changed

+27
-31
lines changed

Diff for: compiler/packages/babel-plugin-react-compiler/src/Optimization/DeadCodeElimination.ts

+15-17
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
*/
77

88
import {
9-
ArrayPattern,
109
BlockId,
1110
HIRFunction,
1211
Identifier,
@@ -184,29 +183,28 @@ function rewriteInstruction(instr: Instruction, state: State): void {
184183
switch (instr.value.lvalue.pattern.kind) {
185184
case 'ArrayPattern': {
186185
/*
187-
* For arrays, we can only eliminate unused items from the end of the array,
188-
* so we iterate from the end and break once we find a used item. Note that
189-
* we already know at least one item is used, from the pruneableValue check.
186+
* For arrays, we can prune items prior to the end by replacing
187+
* them with a hole. Items at the end can simply be dropped.
190188
*/
191-
let nextItems: ArrayPattern['items'] | null = null;
192-
const originalItems = instr.value.lvalue.pattern.items;
193-
for (let i = originalItems.length - 1; i >= 0; i--) {
194-
const item = originalItems[i];
189+
let lastEntryIndex = 0;
190+
const items = instr.value.lvalue.pattern.items;
191+
for (let i = 0; i < items.length; i++) {
192+
const item = items[i];
195193
if (item.kind === 'Identifier') {
196-
if (state.isIdOrNameUsed(item.identifier)) {
197-
nextItems = originalItems.slice(0, i + 1);
198-
break;
194+
if (!state.isIdOrNameUsed(item.identifier)) {
195+
items[i] = {kind: 'Hole'};
196+
} else {
197+
lastEntryIndex = i;
199198
}
200199
} else if (item.kind === 'Spread') {
201-
if (state.isIdOrNameUsed(item.place.identifier)) {
202-
nextItems = originalItems.slice(0, i + 1);
203-
break;
200+
if (!state.isIdOrNameUsed(item.place.identifier)) {
201+
items[i] = {kind: 'Hole'};
202+
} else {
203+
lastEntryIndex = i;
204204
}
205205
}
206206
}
207-
if (nextItems !== null) {
208-
instr.value.lvalue.pattern.items = nextItems;
209-
}
207+
items.length = lastEntryIndex + 1;
210208
break;
211209
}
212210
case 'ObjectPattern': {

Diff for: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/concise-arrow-expr.expect.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ function component() {
1616
import { c as _c } from "react/compiler-runtime";
1717
function component() {
1818
const $ = _c(1);
19-
const [x, setX] = useState(0);
19+
const [, setX] = useState(0);
2020
let t0;
2121
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
2222
const handler = (v) => setX(v);

Diff for: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/escape-analysis-destructured-rest-element.expect.md

+1-2
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ function Component(props) {
3535
}
3636
let d;
3737
if ($[2] !== props.c) {
38-
const [c, ...t0] = props.c;
39-
d = t0;
38+
[, ...d] = props.c;
4039
$[2] = props.c;
4140
$[3] = d;
4241
} else {

Diff for: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inadvertent-mutability-readonly-lambda.expect.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ function Component(props) {
2424
import { c as _c } from "react/compiler-runtime";
2525
function Component(props) {
2626
const $ = _c(2);
27-
const [value, setValue] = useState(null);
27+
const [, setValue] = useState(null);
2828
let t0;
2929
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
3030
t0 = (e) => setValue((value_0) => value_0 + e.target.value);

Diff for: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-calls-to-hoisted-callback-from-other-callback.expect.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import { useState } from "react";
3939

4040
function Component(props) {
4141
const $ = _c(1);
42-
const [_state, setState] = useState();
42+
const [, setState] = useState();
4343
let t0;
4444
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
4545
const a = () => b();

Diff for: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/preserve-use-memo-transition.expect.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import { useCallback, useTransition } from "react";
2828

2929
function useFoo() {
3030
const $ = _c(1);
31-
const [t, start] = useTransition();
31+
const [, start] = useTransition();
3232
let t0;
3333
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
3434
t0 = () => {

Diff for: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/react-namespace.expect.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ function Component(props) {
3232
const $ = _c(5);
3333
React.useContext(FooContext);
3434
const ref = React.useRef();
35-
const [x, setX] = React.useState(false);
35+
const [, setX] = React.useState(false);
3636
let t0;
3737
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
3838
t0 = () => {

Diff for: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-phi-setState-type.expect.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ import { useState } from "react";
6161

6262
function Component(props) {
6363
const $ = _c(5);
64-
const [x, setX] = useState(false);
65-
const [y, setY] = useState(false);
64+
const [, setX] = useState(false);
65+
const [, setY] = useState(false);
6666
let setState;
6767
if (props.cond) {
6868
setState = setX;

Diff for: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-undefined-expression-of-jsxexpressioncontainer.expect.md

+1-2
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,7 @@ function Component(props) {
5252
const { buttons } = props;
5353
let nonPrimaryButtons;
5454
if ($[0] !== buttons) {
55-
const [primaryButton, ...t0] = buttons;
56-
nonPrimaryButtons = t0;
55+
[, ...nonPrimaryButtons] = buttons;
5756
$[0] = buttons;
5857
$[1] = nonPrimaryButtons;
5958
} else {

Diff for: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unused-array-middle-element.expect.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export const FIXTURE_ENTRYPOINT = {
1919

2020
```javascript
2121
function foo(props) {
22-
const [x, unused, y] = props.a;
22+
const [x, , y] = props.a;
2323
return x + y;
2424
}
2525

Diff for: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useActionState-dispatch-considered-as-non-reactive.expect.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import { useActionState } from "react";
2929

3030
function Component() {
3131
const $ = _c(1);
32-
const [actionState, dispatchAction] = useActionState();
32+
const [, dispatchAction] = useActionState();
3333
let t0;
3434
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
3535
const onSubmitAction = () => {

Diff for: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import { useReducer } from "react";
3030

3131
function f() {
3232
const $ = _c(1);
33-
const [state, dispatch] = useReducer();
33+
const [, dispatch] = useReducer();
3434
let t0;
3535
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
3636
const onClick = () => {

0 commit comments

Comments
 (0)