Skip to content

Commit 85f6336

Browse files
committed
[compiler] Handle sequential optionals
The previous PR (facebook#35606) handled maybe-throw terminals within value blocks, and the changes there also made BuildReactiveFunction able to handle sequential optionals. However, CollectOptionalChainDependencies doesn't yet handle sequential value blocks, like `foo(a?.b, b?.c) ?? {}`. This PR fixes the remaining case, which means we should no longer have arbitrary limitations around value blocks.
1 parent b8a6bfa commit 85f6336

File tree

41 files changed

+761
-82
lines changed

Some content is hidden

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

41 files changed

+761
-82
lines changed

compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts

Lines changed: 121 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,121 @@ function matchOptionalTestBlock(
226226
return null;
227227
}
228228

229+
/**
230+
* Finds the test block for an optional chain by traversing through intermediate
231+
* value blocks and nested value block terminals. This handles cases like
232+
* `foo(a?.value, b?.value)?.result` where multiple optional chains or other
233+
* value blocks (logicals, ternaries) appear as arguments, causing the fallthrough
234+
* of one value block to contain another value block terminal rather than directly
235+
* containing a branch.
236+
*
237+
* This function also ensures any nested optional chains encountered are properly
238+
* traversed and added to the context.
239+
*/
240+
function findOptionalTestBlock(
241+
blockId: BlockId,
242+
context: OptionalTraversalContext,
243+
): BasicBlock | null {
244+
const block = context.blocks.get(blockId);
245+
if (block == null) {
246+
return null;
247+
}
248+
249+
const terminal = block.terminal;
250+
switch (terminal.kind) {
251+
case 'branch': {
252+
// Found the branch terminal we're looking for
253+
return block;
254+
}
255+
case 'optional': {
256+
// Traverse the nested optional to collect its dependencies
257+
if (!context.seenOptionals.has(block.id)) {
258+
traverseOptionalBlock(
259+
block as TBasicBlock<OptionalTerminal>,
260+
context,
261+
null,
262+
);
263+
}
264+
// Continue searching in the fallthrough of this optional
265+
return findOptionalTestBlock(terminal.fallthrough, context);
266+
}
267+
case 'logical':
268+
case 'ternary': {
269+
// Traverse nested value blocks in the test branch
270+
const testBlock = context.blocks.get(terminal.test);
271+
if (testBlock?.terminal.kind === 'branch') {
272+
traverseValueBlock(testBlock.terminal.consequent, context);
273+
traverseValueBlock(testBlock.terminal.alternate, context);
274+
}
275+
// Continue searching in the fallthrough
276+
return findOptionalTestBlock(terminal.fallthrough, context);
277+
}
278+
case 'sequence': {
279+
// Traverse the sequence block for any nested optionals
280+
traverseValueBlock(terminal.block, context);
281+
// Continue searching in the fallthrough
282+
return findOptionalTestBlock(terminal.fallthrough, context);
283+
}
284+
default: {
285+
// Unexpected terminal kind - return the block so caller can handle/error
286+
return block;
287+
}
288+
}
289+
}
290+
291+
/**
292+
* Traverses a value block looking for nested optional chains to collect.
293+
* This handles cases where optional chains appear inside logical, ternary,
294+
* or sequence expressions.
295+
*/
296+
function traverseValueBlock(
297+
blockId: BlockId,
298+
context: OptionalTraversalContext,
299+
): void {
300+
const block = context.blocks.get(blockId);
301+
if (block == null) {
302+
return;
303+
}
304+
305+
const terminal = block.terminal;
306+
switch (terminal.kind) {
307+
case 'optional': {
308+
if (!context.seenOptionals.has(block.id)) {
309+
traverseOptionalBlock(
310+
block as TBasicBlock<OptionalTerminal>,
311+
context,
312+
null,
313+
);
314+
}
315+
break;
316+
}
317+
case 'logical':
318+
case 'ternary': {
319+
const testBlock = context.blocks.get(terminal.test);
320+
if (testBlock?.terminal.kind === 'branch') {
321+
traverseValueBlock(testBlock.terminal.consequent, context);
322+
traverseValueBlock(testBlock.terminal.alternate, context);
323+
}
324+
traverseValueBlock(terminal.fallthrough, context);
325+
break;
326+
}
327+
case 'sequence': {
328+
traverseValueBlock(terminal.block, context);
329+
traverseValueBlock(terminal.fallthrough, context);
330+
break;
331+
}
332+
case 'branch':
333+
case 'goto': {
334+
// Terminal blocks, nothing more to traverse
335+
break;
336+
}
337+
default: {
338+
// Other terminal kinds - don't recurse
339+
break;
340+
}
341+
}
342+
}
343+
229344
/**
230345
* Traverse into the optional block and all transitively referenced blocks to
231346
* collect sidemaps of optional chain dependencies.
@@ -302,15 +417,18 @@ function traverseOptionalBlock(
302417
* - <inner_optional> <other operation>
303418
* - a optional base block with a separate nested optional-chain (e.g. a(c?.d)?.d)
304419
*/
305-
const testBlock = context.blocks.get(maybeTest.terminal.fallthrough)!;
306-
if (testBlock!.terminal.kind !== 'branch') {
420+
const testBlock = findOptionalTestBlock(
421+
maybeTest.terminal.fallthrough,
422+
context,
423+
);
424+
if (testBlock == null || testBlock.terminal.kind !== 'branch') {
307425
/**
308426
* Fallthrough of the inner optional should be a block with no
309427
* instructions, terminating with Test($<temporary written to from
310428
* StoreLocal>)
311429
*/
312430
CompilerError.throwTodo({
313-
reason: `Unexpected terminal kind \`${testBlock.terminal.kind}\` for optional fallthrough block`,
431+
reason: `Unexpected terminal kind \`${testBlock?.terminal.kind ?? '(null)'}\` for optional fallthrough block`,
314432
loc: maybeTest.terminal.loc,
315433
});
316434
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
2+
## Input
3+
4+
```javascript
5+
function Component({a, b}) {
6+
'use memo';
7+
return useHook(a?.value, b?.value) ?? {};
8+
}
9+
10+
```
11+
12+
## Code
13+
14+
```javascript
15+
function Component(t0) {
16+
"use memo";
17+
const { a, b } = t0;
18+
19+
return useHook(a?.value, b?.value) ?? {};
20+
}
21+
22+
```
23+
24+
### Eval output
25+
(kind: exception) Fixture not implemented
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
function Component({a, b}) {
2+
'use memo';
3+
return useHook(a?.value, b?.value) ?? {};
4+
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-optional-call-chain-in-optional.expect.md

Lines changed: 0 additions & 39 deletions
This file was deleted.
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
2+
## Input
3+
4+
```javascript
5+
function useFoo(props: {value: {x: string; y: string} | null}) {
6+
const value = props.value;
7+
return createArray(value?.x, value?.y)?.join(', ');
8+
}
9+
10+
function createArray<T>(...args: Array<T>): Array<T> {
11+
return args;
12+
}
13+
14+
export const FIXTURE_ENTRYPONT = {
15+
fn: useFoo,
16+
props: [{value: null}],
17+
};
18+
19+
```
20+
21+
## Code
22+
23+
```javascript
24+
import { c as _c } from "react/compiler-runtime";
25+
function useFoo(props) {
26+
const $ = _c(3);
27+
const value = props.value;
28+
let t0;
29+
if ($[0] !== value?.x || $[1] !== value?.y) {
30+
t0 = createArray(value?.x, value?.y)?.join(", ");
31+
$[0] = value?.x;
32+
$[1] = value?.y;
33+
$[2] = t0;
34+
} else {
35+
t0 = $[2];
36+
}
37+
return t0;
38+
}
39+
40+
function createArray(...t0) {
41+
const args = t0;
42+
return args;
43+
}
44+
45+
export const FIXTURE_ENTRYPONT = {
46+
fn: useFoo,
47+
props: [{ value: null }],
48+
};
49+
50+
```
51+
52+
### Eval output
53+
(kind: exception) Fixture not implemented

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-optional-call-chain-in-optional.ts renamed to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-call-chain-in-optional.ts

File renamed without changes.
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
2+
## Input
3+
4+
```javascript
5+
function Component({a, b}) {
6+
return foo(a?.value, b?.value)?.result;
7+
}
8+
9+
```
10+
11+
## Code
12+
13+
```javascript
14+
function Component(t0) {
15+
const { a, b } = t0;
16+
return foo(a?.value, b?.value)?.result;
17+
}
18+
19+
```
20+
21+
### Eval output
22+
(kind: exception) Fixture not implemented
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
function Component({a, b}) {
2+
return foo(a?.value, b?.value)?.result;
3+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
2+
## Input
3+
4+
```javascript
5+
// Mix of optional chain and logical AND in call args
6+
function Component({a, b, c}) {
7+
return foo(a?.value, b && b.value, c?.value)?.result;
8+
}
9+
10+
```
11+
12+
## Code
13+
14+
```javascript
15+
// Mix of optional chain and logical AND in call args
16+
function Component(t0) {
17+
const { a, b, c } = t0;
18+
return foo(a?.value, b && b.value, c?.value)?.result;
19+
}
20+
21+
```
22+
23+
### Eval output
24+
(kind: exception) Fixture not implemented
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// Mix of optional chain and logical AND in call args
2+
function Component({a, b, c}) {
3+
return foo(a?.value, b && b.value, c?.value)?.result;
4+
}

0 commit comments

Comments
 (0)