Skip to content

Commit 2e5dc27

Browse files
committed
[wip][compiler] Infer optional dependencies
Updates PropagateScopeDeps and DeriveMinimalDeps to understand optional dependency paths (`a?.b`). There a few key pieces to this: In PropagateScopeDeps we jump through some hoops to work around the awkward structure of nested OptionalExpressions. This is much easier in HIR form, but I managed to get this pretty close and i think it will be landable with further cleanup. A good chunk of this is avoiding prematurely registering a value as a dependency - there are a bunch of indirections in the ReactiveFunction structure: ``` t0 = OptionalExpression SequenceExpression t0 = Sequence ... LoadLocal t0 ``` Where if at any point we call `visitOperand()` we'll prematurely register a dependency instead of declareProperty(). The other bit is that optionals can be optional=false for nested member expressions where not all the parts are actually optional (`foo.bar?.bar.call()`). And of course, parts of an optional chain can still be conditional even when optional=true (for example the `x` in `foo.bar?.[x]?.baz`). Not all of this is tested yet so there are likely bugs still. The other bit is DeriveMinimalDeps, which is thankfully easier. We add OptionalAccess and OptionalDep and update the merge and reducing logic for these cases. There is probably still more to update though, for things like merging subtrees. There are a lot of ternaries that assume a result can be exactly one of two states (conditional/unconditional, dependency/access) and these assumptions don't hold anymore. I'd like to refactor to dependency/access separate from conditional/optional/unconditional. Also, the reducing logic isn't quite right: once a child is optional we keep inferring all the parents as optional too, losing some precision. I need to adjust the reducing logic to let children decide whether their path token is optional or not. ghstack-source-id: a04946e Pull Request resolved: #30819
1 parent ac01cf7 commit 2e5dc27

13 files changed

+255
-74
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ export function printTerminal(terminal: Terminal): Array<string> | string {
191191
case 'branch': {
192192
value = `[${terminal.id}] Branch (${printPlace(terminal.test)}) then:bb${
193193
terminal.consequent
194-
} else:bb${terminal.alternate}`;
194+
} else:bb${terminal.alternate} fallthrough:bb${terminal.fallthrough}`;
195195
break;
196196
}
197197
case 'logical': {

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,9 +1446,19 @@ function codegenDependency(
14461446
dependency: ReactiveScopeDependency,
14471447
): t.Expression {
14481448
let object: t.Expression = convertIdentifier(dependency.identifier);
1449-
if (dependency.path !== null) {
1449+
if (dependency.path.length !== 0) {
1450+
const hasOptional = dependency.path.some(path => path.optional);
14501451
for (const path of dependency.path) {
1451-
object = t.memberExpression(object, t.identifier(path.property));
1452+
if (hasOptional) {
1453+
object = t.optionalMemberExpression(
1454+
object,
1455+
t.identifier(path.property),
1456+
false,
1457+
path.optional,
1458+
);
1459+
} else {
1460+
object = t.memberExpression(object, t.identifier(path.property));
1461+
}
14521462
}
14531463
}
14541464
return object;

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/DeriveMinimalDependencies.ts

Lines changed: 81 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {CompilerError} from '../CompilerError';
99
import {DependencyPath, Identifier, ReactiveScopeDependency} from '../HIR';
1010
import {printIdentifier} from '../HIR/PrintHIR';
1111
import {assertExhaustive} from '../Utils/utils';
12+
import {printDependency} from './PrintReactiveFunction';
1213

1314
/*
1415
* We need to understand optional member expressions only when determining
@@ -60,13 +61,14 @@ export class ReactiveScopeDependencyTree {
6061
const {path} = dep;
6162
let currNode = this.#getOrCreateRoot(dep.identifier);
6263

63-
const accessType = inConditional
64-
? PropertyAccessType.ConditionalAccess
65-
: PropertyAccessType.UnconditionalAccess;
66-
6764
for (const item of path) {
6865
// all properties read 'on the way' to a dependency are marked as 'access'
6966
let currChild = getOrMakeProperty(currNode, item.property);
67+
const accessType = inConditional
68+
? PropertyAccessType.ConditionalAccess
69+
: item.optional
70+
? PropertyAccessType.OptionalAccess
71+
: PropertyAccessType.UnconditionalAccess;
7072
currChild.accessType = merge(currChild.accessType, accessType);
7173
currNode = currChild;
7274
}
@@ -77,7 +79,9 @@ export class ReactiveScopeDependencyTree {
7779
*/
7880
const depType = inConditional
7981
? PropertyAccessType.ConditionalDependency
80-
: PropertyAccessType.UnconditionalDependency;
82+
: isOptional(currNode.accessType)
83+
? PropertyAccessType.OptionalDependency
84+
: PropertyAccessType.UnconditionalDependency;
8185

8286
currNode.accessType = merge(currNode.accessType, depType);
8387
}
@@ -88,7 +92,9 @@ export class ReactiveScopeDependencyTree {
8892
const deps = deriveMinimalDependenciesInSubtree(rootNode);
8993
CompilerError.invariant(
9094
deps.every(
91-
dep => dep.accessType === PropertyAccessType.UnconditionalDependency,
95+
dep =>
96+
dep.accessType === PropertyAccessType.UnconditionalDependency ||
97+
dep.accessType == PropertyAccessType.OptionalDependency,
9298
),
9399
{
94100
reason:
@@ -173,6 +179,27 @@ export class ReactiveScopeDependencyTree {
173179
}
174180
return res.flat().join('\n');
175181
}
182+
183+
debug(): string {
184+
const buf: Array<string> = [`tree() [`];
185+
for (const [rootId, rootNode] of this.#roots) {
186+
buf.push(`${printIdentifier(rootId)} (${rootNode.accessType}):`);
187+
this.#debugImpl(buf, rootNode, 1);
188+
}
189+
buf.push(']');
190+
return buf.length > 2 ? buf.join('\n') : buf.join('');
191+
}
192+
193+
#debugImpl(
194+
buf: Array<string>,
195+
node: DependencyNode,
196+
depth: number = 0,
197+
): void {
198+
for (const [property, childNode] of node.properties) {
199+
buf.push(`${' '.repeat(depth)}.${property} (${childNode.accessType}):`);
200+
this.#debugImpl(buf, childNode, depth + 1);
201+
}
202+
}
176203
}
177204

178205
/*
@@ -196,8 +223,10 @@ export class ReactiveScopeDependencyTree {
196223
*/
197224
enum PropertyAccessType {
198225
ConditionalAccess = 'ConditionalAccess',
226+
OptionalAccess = 'OptionalAccess',
199227
UnconditionalAccess = 'UnconditionalAccess',
200228
ConditionalDependency = 'ConditionalDependency',
229+
OptionalDependency = 'OptionalDependency',
201230
UnconditionalDependency = 'UnconditionalDependency',
202231
}
203232

@@ -211,9 +240,16 @@ function isUnconditional(access: PropertyAccessType): boolean {
211240
function isDependency(access: PropertyAccessType): boolean {
212241
return (
213242
access === PropertyAccessType.ConditionalDependency ||
243+
access === PropertyAccessType.OptionalDependency ||
214244
access === PropertyAccessType.UnconditionalDependency
215245
);
216246
}
247+
function isOptional(access: PropertyAccessType): boolean {
248+
return (
249+
access === PropertyAccessType.OptionalAccess ||
250+
access === PropertyAccessType.OptionalDependency
251+
);
252+
}
217253

218254
function merge(
219255
access1: PropertyAccessType,
@@ -222,6 +258,7 @@ function merge(
222258
const resultIsUnconditional =
223259
isUnconditional(access1) || isUnconditional(access2);
224260
const resultIsDependency = isDependency(access1) || isDependency(access2);
261+
const resultIsOptional = isOptional(access1) || isOptional(access2);
225262

226263
/*
227264
* Straightforward merge.
@@ -237,6 +274,12 @@ function merge(
237274
} else {
238275
return PropertyAccessType.UnconditionalAccess;
239276
}
277+
} else if (resultIsOptional) {
278+
if (resultIsDependency) {
279+
return PropertyAccessType.OptionalDependency;
280+
} else {
281+
return PropertyAccessType.OptionalAccess;
282+
}
240283
} else {
241284
if (resultIsDependency) {
242285
return PropertyAccessType.ConditionalDependency;
@@ -256,19 +299,25 @@ type ReduceResultNode = {
256299
accessType: PropertyAccessType;
257300
};
258301

259-
const promoteUncondResult = [
302+
const promoteUncondResult: Array<ReduceResultNode> = [
260303
{
261304
relativePath: [],
262305
accessType: PropertyAccessType.UnconditionalDependency,
263306
},
264307
];
265308

266-
const promoteCondResult = [
309+
const promoteCondResult: Array<ReduceResultNode> = [
267310
{
268311
relativePath: [],
269312
accessType: PropertyAccessType.ConditionalDependency,
270313
},
271314
];
315+
const promoteOptionalResult: Array<ReduceResultNode> = [
316+
{
317+
relativePath: [],
318+
accessType: PropertyAccessType.OptionalDependency,
319+
},
320+
];
272321

273322
/*
274323
* Recursively calculates minimal dependencies in a subtree.
@@ -284,7 +333,7 @@ function deriveMinimalDependenciesInSubtree(
284333
({relativePath, accessType}) => {
285334
return {
286335
relativePath: [
287-
{property: childName, optional: false},
336+
{property: childName, optional: isOptional(accessType)},
288337
...relativePath,
289338
],
290339
accessType,
@@ -302,7 +351,8 @@ function deriveMinimalDependenciesInSubtree(
302351
if (
303352
results.every(
304353
({accessType}) =>
305-
accessType === PropertyAccessType.UnconditionalDependency,
354+
accessType === PropertyAccessType.UnconditionalDependency ||
355+
accessType === PropertyAccessType.OptionalDependency,
306356
)
307357
) {
308358
// all children are unconditional dependencies, return them to preserve granularity
@@ -315,6 +365,27 @@ function deriveMinimalDependenciesInSubtree(
315365
return promoteUncondResult;
316366
}
317367
}
368+
case PropertyAccessType.OptionalDependency: {
369+
return promoteOptionalResult;
370+
}
371+
case PropertyAccessType.OptionalAccess: {
372+
if (
373+
results.every(
374+
({accessType}) =>
375+
accessType === PropertyAccessType.UnconditionalDependency ||
376+
accessType === PropertyAccessType.OptionalDependency,
377+
)
378+
) {
379+
// all children are unconditional dependencies, return them to preserve granularity
380+
return results;
381+
} else {
382+
/*
383+
* at least one child is accessed conditionally, so this node needs to be promoted to
384+
* unconditional dependency
385+
*/
386+
return promoteOptionalResult;
387+
}
388+
}
318389
case PropertyAccessType.ConditionalAccess:
319390
case PropertyAccessType.ConditionalDependency: {
320391
if (

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ export function printDependency(dependency: ReactiveScopeDependency): string {
113113
const identifier =
114114
printIdentifier(dependency.identifier) +
115115
printType(dependency.identifier.type);
116-
return `${identifier}${dependency.path.map(token => `.${token.property}`).join('')}`;
116+
return `${identifier}${dependency.path.map(token => `${token.optional ? '?.' : '.'}${token.property}`).join('')}`;
117117
}
118118

119119
export function printReactiveInstructions(

0 commit comments

Comments
 (0)