Skip to content

[compiler] Handle sequential optionals #35673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
josephsavona wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[compiler] Handle sequential optionals #35673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 121 additions & 3 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,121 @@ function matchOptionalTestBlock(
return null;
}

/**
* Finds the test block for an optional chain by traversing through intermediate
* value blocks and nested value block terminals. This handles cases like
* `foo(a?.value, b?.value)?.result` where multiple optional chains or other
* value blocks (logicals, ternaries) appear as arguments, causing the fallthrough
* of one value block to contain another value block terminal rather than directly
* containing a branch.
*
* This function also ensures any nested optional chains encountered are properly
* traversed and added to the context.
*/
function findOptionalTestBlock(
blockId: BlockId,
context: OptionalTraversalContext,
): BasicBlock | null {
const block = context.blocks.get(blockId);
if (block == null) {
return null;
}

const terminal = block.terminal;
switch (terminal.kind) {
case 'branch': {
// Found the branch terminal we're looking for
return block;
}
case 'optional': {
// Traverse the nested optional to collect its dependencies
if (!context.seenOptionals.has(block.id)) {
traverseOptionalBlock(
block as TBasicBlock<OptionalTerminal>,
context,
null,
);
}
// Continue searching in the fallthrough of this optional
return findOptionalTestBlock(terminal.fallthrough, context);
}
case 'logical':
case 'ternary': {
// Traverse nested value blocks in the test branch
const testBlock = context.blocks.get(terminal.test);
if (testBlock?.terminal.kind === 'branch') {
traverseValueBlock(testBlock.terminal.consequent, context);
traverseValueBlock(testBlock.terminal.alternate, context);
}
// Continue searching in the fallthrough
return findOptionalTestBlock(terminal.fallthrough, context);
}
case 'sequence': {
// Traverse the sequence block for any nested optionals
traverseValueBlock(terminal.block, context);
// Continue searching in the fallthrough
return findOptionalTestBlock(terminal.fallthrough, context);
}
default: {
// Unexpected terminal kind - return the block so caller can handle/error
return block;
}
}
}

/**
* Traverses a value block looking for nested optional chains to collect.
* This handles cases where optional chains appear inside logical, ternary,
* or sequence expressions.
*/
function traverseValueBlock(
blockId: BlockId,
context: OptionalTraversalContext,
): void {
const block = context.blocks.get(blockId);
if (block == null) {
return;
}

const terminal = block.terminal;
switch (terminal.kind) {
case 'optional': {
if (!context.seenOptionals.has(block.id)) {
traverseOptionalBlock(
block as TBasicBlock<OptionalTerminal>,
context,
null,
);
}
break;
}
case 'logical':
case 'ternary': {
const testBlock = context.blocks.get(terminal.test);
if (testBlock?.terminal.kind === 'branch') {
traverseValueBlock(testBlock.terminal.consequent, context);
traverseValueBlock(testBlock.terminal.alternate, context);
}
traverseValueBlock(terminal.fallthrough, context);
break;
}
case 'sequence': {
traverseValueBlock(terminal.block, context);
traverseValueBlock(terminal.fallthrough, context);
break;
}
case 'branch':
case 'goto': {
// Terminal blocks, nothing more to traverse
break;
}
default: {
// Other terminal kinds - don't recurse
break;
}
}
}

/**
* Traverse into the optional block and all transitively referenced blocks to
* collect sidemaps of optional chain dependencies.
Expand Down Expand Up @@ -302,15 +417,18 @@ function traverseOptionalBlock(
* - <inner_optional> <other operation>
* - a optional base block with a separate nested optional-chain (e.g. a(c?.d)?.d)
*/
const testBlock = context.blocks.get(maybeTest.terminal.fallthrough)!;
if (testBlock!.terminal.kind !== 'branch') {
const testBlock = findOptionalTestBlock(
maybeTest.terminal.fallthrough,
context,
);
if (testBlock == null || testBlock.terminal.kind !== 'branch') {
/**
* Fallthrough of the inner optional should be a block with no
* instructions, terminating with Test($<temporary written to from
* StoreLocal>)
*/
CompilerError.throwTodo({
reason: `Unexpected terminal kind \`${testBlock.terminal.kind}\` for optional fallthrough block`,
reason: `Unexpected terminal kind \`${testBlock?.terminal.kind ?? '(null)'}\` for optional fallthrough block`,
loc: maybeTest.terminal.loc,
});
}
Expand Down
25 changes: 25 additions & 0 deletions ...src/__tests__/fixtures/compiler/bug-nested-optional-member-in-logical.expect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@

## Input

```javascript
function Component({a, b}) {
'use memo';
return useHook(a?.value, b?.value) ?? {};
}

```

## Code

```javascript
function Component(t0) {
"use memo";
const { a, b } = t0;

return useHook(a?.value, b?.value) ?? {};
}

```

### Eval output
(kind: exception) Fixture not implemented
4 changes: 4 additions & 0 deletions ...n-react-compiler/src/__tests__/fixtures/compiler/bug-nested-optional-member-in-logical.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
function Component({a, b}) {
'use memo';
return useHook(a?.value, b?.value) ?? {};
}
39 changes: 0 additions & 39 deletions ..._tests__/fixtures/compiler/error.todo-optional-call-chain-in-optional.expect.md
View file
Open in desktop

This file was deleted.

53 changes: 53 additions & 0 deletions ...piler/src/__tests__/fixtures/compiler/optional-call-chain-in-optional.expect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@

## Input

```javascript
function useFoo(props: {value: {x: string; y: string} | null}) {
const value = props.value;
return createArray(value?.x, value?.y)?.join(', ');
}

function createArray<T>(...args: Array<T>): Array<T> {
return args;
}

export const FIXTURE_ENTRYPONT = {
fn: useFoo,
props: [{value: null}],
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime";
function useFoo(props) {
const $ = _c(3);
const value = props.value;
let t0;
if ($[0] !== value?.x || $[1] !== value?.y) {
t0 = createArray(value?.x, value?.y)?.join(", ");
$[0] = value?.x;
$[1] = value?.y;
$[2] = t0;
} else {
t0 = $[2];
}
return t0;
}

function createArray(...t0) {
const args = t0;
return args;
}

export const FIXTURE_ENTRYPONT = {
fn: useFoo,
props: [{ value: null }],
};

```

### Eval output
(kind: exception) Fixture not implemented
0 ...r.todo-optional-call-chain-in-optional.ts → ...mpiler/optional-call-chain-in-optional.ts
View file
Open in desktop
File renamed without changes.
22 changes: 22 additions & 0 deletions ...__tests__/fixtures/compiler/optional-call-with-multiple-optional-args.expect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@

## Input

```javascript
function Component({a, b}) {
return foo(a?.value, b?.value)?.result;
}

```

## Code

```javascript
function Component(t0) {
const { a, b } = t0;
return foo(a?.value, b?.value)?.result;
}

```

### Eval output
(kind: exception) Fixture not implemented
3 changes: 3 additions & 0 deletions ...act-compiler/src/__tests__/fixtures/compiler/optional-call-with-multiple-optional-args.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
function Component({a, b}) {
return foo(a?.value, b?.value)?.result;
}
24 changes: 24 additions & 0 deletions ...er/src/__tests__/fixtures/compiler/optional-chain-and-logical-in-call.expect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@

## Input

```javascript
// Mix of optional chain and logical AND in call args
function Component({a, b, c}) {
return foo(a?.value, b && b.value, c?.value)?.result;
}

```

## Code

```javascript
// Mix of optional chain and logical AND in call args
function Component(t0) {
const { a, b, c } = t0;
return foo(a?.value, b && b.value, c?.value)?.result;
}

```

### Eval output
(kind: exception) Fixture not implemented
4 changes: 4 additions & 0 deletions ...ugin-react-compiler/src/__tests__/fixtures/compiler/optional-chain-and-logical-in-call.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Mix of optional chain and logical AND in call args
function Component({a, b, c}) {
return foo(a?.value, b && b.value, c?.value)?.result;
}
24 changes: 24 additions & 0 deletions ...ler/src/__tests__/fixtures/compiler/optional-chain-deeply-nested-args.expect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@

## Input

```javascript
// Deeply nested optional chains as args
function Component({a, b}) {
return foo(a?.b?.c?.d, b?.e?.f?.g)?.result?.final;
}

```

## Code

```javascript
// Deeply nested optional chains as args
function Component(t0) {
const { a, b } = t0;
return foo(a?.b?.c?.d, b?.e?.f?.g)?.result?.final;
}

```

### Eval output
(kind: exception) Fixture not implemented
4 changes: 4 additions & 0 deletions ...lugin-react-compiler/src/__tests__/fixtures/compiler/optional-chain-deeply-nested-args.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Deeply nested optional chains as args
function Component({a, b}) {
return foo(a?.b?.c?.d, b?.e?.f?.g)?.result?.final;
}
35 changes: 35 additions & 0 deletions ...s__/fixtures/compiler/optional-chain-double-method-with-optional-args.expect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@

## Input

```javascript
// Chained optional method calls with optional args
function Component({obj, a, b, c}) {
return obj(a?.value)?.second(b?.value);
}

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime"; // Chained optional method calls with optional args
function Component(t0) {
const $ = _c(4);
const { obj, a, b } = t0;
let t1;
if ($[0] !== a?.value || $[1] !== b?.value || $[2] !== obj) {
t1 = obj(a?.value)?.second(b?.value);
$[0] = a?.value;
$[1] = b?.value;
$[2] = obj;
$[3] = t1;
} else {
t1 = $[3];
}
return t1;
}

```

### Eval output
(kind: exception) Fixture not implemented
Loading
Loading