Skip to content

Commit 6fc33d7

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: 2a8817d Pull Request resolved: #30819
1 parent ac01cf7 commit 6fc33d7

16 files changed

+439
-140
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,14 @@ const EnvironmentConfigSchema = z.object({
224224

225225
enableReactiveScopesInHIR: z.boolean().default(true),
226226

227+
/**
228+
* Enables inference of optional dependency chains. Without this flag
229+
* a property chain such as `props?.items?.foo` will infer as a dep on
230+
* just `props`. With this flag enabled, we'll infer that full path as
231+
* the dependency.
232+
*/
233+
enableOptionalDependencies: z.boolean().default(false),
234+
227235
/*
228236
* Enable validation of hooks to partially check that the component honors the rules of hooks.
229237
* When disabled, the component is assumed to follow the rules (though the Babel plugin looks

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: 131 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,14 @@ export class ReactiveScopeDependencyTree {
6060
const {path} = dep;
6161
let currNode = this.#getOrCreateRoot(dep.identifier);
6262

63-
const accessType = inConditional
64-
? PropertyAccessType.ConditionalAccess
65-
: PropertyAccessType.UnconditionalAccess;
66-
6763
for (const item of path) {
6864
// all properties read 'on the way' to a dependency are marked as 'access'
6965
let currChild = getOrMakeProperty(currNode, item.property);
66+
const accessType = inConditional
67+
? PropertyAccessType.ConditionalAccess
68+
: item.optional
69+
? PropertyAccessType.OptionalAccess
70+
: PropertyAccessType.UnconditionalAccess;
7071
currChild.accessType = merge(currChild.accessType, accessType);
7172
currNode = currChild;
7273
}
@@ -77,18 +78,22 @@ export class ReactiveScopeDependencyTree {
7778
*/
7879
const depType = inConditional
7980
? PropertyAccessType.ConditionalDependency
80-
: PropertyAccessType.UnconditionalDependency;
81+
: isOptional(currNode.accessType)
82+
? PropertyAccessType.OptionalDependency
83+
: PropertyAccessType.UnconditionalDependency;
8184

8285
currNode.accessType = merge(currNode.accessType, depType);
8386
}
8487

8588
deriveMinimalDependencies(): Set<ReactiveScopeDependency> {
8689
const results = new Set<ReactiveScopeDependency>();
8790
for (const [rootId, rootNode] of this.#roots.entries()) {
88-
const deps = deriveMinimalDependenciesInSubtree(rootNode);
91+
const deps = deriveMinimalDependenciesInSubtree(rootNode, null);
8992
CompilerError.invariant(
9093
deps.every(
91-
dep => dep.accessType === PropertyAccessType.UnconditionalDependency,
94+
dep =>
95+
dep.accessType === PropertyAccessType.UnconditionalDependency ||
96+
dep.accessType == PropertyAccessType.OptionalDependency,
9297
),
9398
{
9499
reason:
@@ -173,6 +178,27 @@ export class ReactiveScopeDependencyTree {
173178
}
174179
return res.flat().join('\n');
175180
}
181+
182+
debug(): string {
183+
const buf: Array<string> = [`tree() [`];
184+
for (const [rootId, rootNode] of this.#roots) {
185+
buf.push(`${printIdentifier(rootId)} (${rootNode.accessType}):`);
186+
this.#debugImpl(buf, rootNode, 1);
187+
}
188+
buf.push(']');
189+
return buf.length > 2 ? buf.join('\n') : buf.join('');
190+
}
191+
192+
#debugImpl(
193+
buf: Array<string>,
194+
node: DependencyNode,
195+
depth: number = 0,
196+
): void {
197+
for (const [property, childNode] of node.properties) {
198+
buf.push(`${' '.repeat(depth)}.${property} (${childNode.accessType}):`);
199+
this.#debugImpl(buf, childNode, depth + 1);
200+
}
201+
}
176202
}
177203

178204
/*
@@ -196,8 +222,10 @@ export class ReactiveScopeDependencyTree {
196222
*/
197223
enum PropertyAccessType {
198224
ConditionalAccess = 'ConditionalAccess',
225+
OptionalAccess = 'OptionalAccess',
199226
UnconditionalAccess = 'UnconditionalAccess',
200227
ConditionalDependency = 'ConditionalDependency',
228+
OptionalDependency = 'OptionalDependency',
201229
UnconditionalDependency = 'UnconditionalDependency',
202230
}
203231

@@ -211,9 +239,16 @@ function isUnconditional(access: PropertyAccessType): boolean {
211239
function isDependency(access: PropertyAccessType): boolean {
212240
return (
213241
access === PropertyAccessType.ConditionalDependency ||
242+
access === PropertyAccessType.OptionalDependency ||
214243
access === PropertyAccessType.UnconditionalDependency
215244
);
216245
}
246+
function isOptional(access: PropertyAccessType): boolean {
247+
return (
248+
access === PropertyAccessType.OptionalAccess ||
249+
access === PropertyAccessType.OptionalDependency
250+
);
251+
}
217252

218253
function merge(
219254
access1: PropertyAccessType,
@@ -222,6 +257,7 @@ function merge(
222257
const resultIsUnconditional =
223258
isUnconditional(access1) || isUnconditional(access2);
224259
const resultIsDependency = isDependency(access1) || isDependency(access2);
260+
const resultIsOptional = isOptional(access1) || isOptional(access2);
225261

226262
/*
227263
* Straightforward merge.
@@ -237,6 +273,12 @@ function merge(
237273
} else {
238274
return PropertyAccessType.UnconditionalAccess;
239275
}
276+
} else if (resultIsOptional) {
277+
if (resultIsDependency) {
278+
return PropertyAccessType.OptionalDependency;
279+
} else {
280+
return PropertyAccessType.OptionalAccess;
281+
}
240282
} else {
241283
if (resultIsDependency) {
242284
return PropertyAccessType.ConditionalDependency;
@@ -256,19 +298,34 @@ type ReduceResultNode = {
256298
accessType: PropertyAccessType;
257299
};
258300

259-
const promoteUncondResult = [
260-
{
301+
function promoteResult(
302+
accessType: PropertyAccessType,
303+
path: {property: string; optional: boolean} | null,
304+
): Array<ReduceResultNode> {
305+
const result: ReduceResultNode = {
261306
relativePath: [],
262-
accessType: PropertyAccessType.UnconditionalDependency,
263-
},
264-
];
307+
accessType,
308+
};
309+
if (path !== null) {
310+
result.relativePath.push(path);
311+
}
312+
return [result];
313+
}
265314

266-
const promoteCondResult = [
267-
{
268-
relativePath: [],
269-
accessType: PropertyAccessType.ConditionalDependency,
270-
},
271-
];
315+
function prependPath(
316+
results: Array<ReduceResultNode>,
317+
path: {property: string; optional: boolean} | null,
318+
): Array<ReduceResultNode> {
319+
if (path === null) {
320+
return results;
321+
}
322+
return results.map(result => {
323+
return {
324+
accessType: result.accessType,
325+
relativePath: [path, ...result.relativePath],
326+
};
327+
});
328+
}
272329

273330
/*
274331
* Recursively calculates minimal dependencies in a subtree.
@@ -277,42 +334,76 @@ const promoteCondResult = [
277334
*/
278335
function deriveMinimalDependenciesInSubtree(
279336
dep: DependencyNode,
337+
property: string | null,
280338
): Array<ReduceResultNode> {
281339
const results: Array<ReduceResultNode> = [];
282340
for (const [childName, childNode] of dep.properties) {
283-
const childResult = deriveMinimalDependenciesInSubtree(childNode).map(
284-
({relativePath, accessType}) => {
285-
return {
286-
relativePath: [
287-
{property: childName, optional: false},
288-
...relativePath,
289-
],
290-
accessType,
291-
};
292-
},
341+
const childResult = deriveMinimalDependenciesInSubtree(
342+
childNode,
343+
childName,
293344
);
294345
results.push(...childResult);
295346
}
296347

297348
switch (dep.accessType) {
298349
case PropertyAccessType.UnconditionalDependency: {
299-
return promoteUncondResult;
350+
return promoteResult(
351+
PropertyAccessType.UnconditionalDependency,
352+
property !== null ? {property, optional: false} : null,
353+
);
300354
}
301355
case PropertyAccessType.UnconditionalAccess: {
302356
if (
303357
results.every(
304358
({accessType}) =>
305-
accessType === PropertyAccessType.UnconditionalDependency,
359+
accessType === PropertyAccessType.UnconditionalDependency ||
360+
accessType === PropertyAccessType.OptionalDependency,
306361
)
307362
) {
308363
// all children are unconditional dependencies, return them to preserve granularity
309-
return results;
364+
return prependPath(
365+
results,
366+
property !== null ? {property, optional: false} : null,
367+
);
310368
} else {
311369
/*
312370
* at least one child is accessed conditionally, so this node needs to be promoted to
313371
* unconditional dependency
314372
*/
315-
return promoteUncondResult;
373+
return promoteResult(
374+
PropertyAccessType.UnconditionalDependency,
375+
property !== null ? {property, optional: false} : null,
376+
);
377+
}
378+
}
379+
case PropertyAccessType.OptionalDependency: {
380+
return promoteResult(
381+
PropertyAccessType.OptionalDependency,
382+
property !== null ? {property, optional: true} : null,
383+
);
384+
}
385+
case PropertyAccessType.OptionalAccess: {
386+
if (
387+
results.every(
388+
({accessType}) =>
389+
accessType === PropertyAccessType.UnconditionalDependency ||
390+
accessType === PropertyAccessType.OptionalDependency,
391+
)
392+
) {
393+
// all children are unconditional dependencies, return them to preserve granularity
394+
return prependPath(
395+
results,
396+
property !== null ? {property, optional: true} : null,
397+
);
398+
} else {
399+
/*
400+
* at least one child is accessed conditionally, so this node needs to be promoted to
401+
* unconditional dependency
402+
*/
403+
return promoteResult(
404+
PropertyAccessType.OptionalDependency,
405+
property !== null ? {property, optional: true} : null,
406+
);
316407
}
317408
}
318409
case PropertyAccessType.ConditionalAccess:
@@ -328,13 +419,19 @@ function deriveMinimalDependenciesInSubtree(
328419
* unconditional access.
329420
* Truncate results of child nodes here, since we shouldn't access them anyways
330421
*/
331-
return promoteCondResult;
422+
return promoteResult(
423+
PropertyAccessType.ConditionalDependency,
424+
property !== null ? {property, optional: true} : null,
425+
);
332426
} else {
333427
/*
334428
* at least one child is accessed unconditionally, so this node can be promoted to
335429
* unconditional dependency
336430
*/
337-
return promoteUncondResult;
431+
return promoteResult(
432+
PropertyAccessType.UnconditionalDependency,
433+
property !== null ? {property, optional: true} : null,
434+
);
338435
}
339436
}
340437
default: {

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import {CompilerError} from '..';
99
import {
1010
DeclarationId,
11-
DependencyPath,
1211
InstructionId,
1312
InstructionKind,
1413
Place,

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)