Skip to content

Commit 90131ce

Browse files
committed
[compiler] Add reactive flag on scope dependencies
When collecting scope dependencies, mark each dependency with `reactive: true | false`. This prepares for later PRs #33326 and #32099 which rewrite scope dependencies into instructions. Note that some reactive objects may have non-reactive properties, but we do not currently track this. Technically, state[0] is reactive and state[1] is not. Currently, both would be marked as reactive. ```js const state = useState(); ```
1 parent 64b555c commit 90131ce

7 files changed

+109
-19
lines changed

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

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,10 @@ type PropertyPathNode =
241241
class PropertyPathRegistry {
242242
roots: Map<IdentifierId, RootNode> = new Map();
243243

244-
getOrCreateIdentifier(identifier: Identifier): PropertyPathNode {
244+
getOrCreateIdentifier(
245+
identifier: Identifier,
246+
reactive: boolean,
247+
): PropertyPathNode {
245248
/**
246249
* Reads from a statically scoped variable are always safe in JS,
247250
* with the exception of TDZ (not addressed by this pass).
@@ -255,12 +258,19 @@ class PropertyPathRegistry {
255258
optionalProperties: new Map(),
256259
fullPath: {
257260
identifier,
261+
reactive,
258262
path: [],
259263
},
260264
hasOptional: false,
261265
parent: null,
262266
};
263267
this.roots.set(identifier.id, rootNode);
268+
} else {
269+
CompilerError.invariant(reactive === rootNode.fullPath.reactive, {
270+
reason:
271+
'[HoistablePropertyLoads] Found inconsistencies in `reactive` flag when deduping identifier reads within the same scope',
272+
loc: identifier.loc,
273+
});
264274
}
265275
return rootNode;
266276
}
@@ -278,6 +288,7 @@ class PropertyPathRegistry {
278288
parent: parent,
279289
fullPath: {
280290
identifier: parent.fullPath.identifier,
291+
reactive: parent.fullPath.reactive,
281292
path: parent.fullPath.path.concat(entry),
282293
},
283294
hasOptional: parent.hasOptional || entry.optional,
@@ -293,7 +304,7 @@ class PropertyPathRegistry {
293304
* so all subpaths of a PropertyLoad should already exist
294305
* (e.g. a.b is added before a.b.c),
295306
*/
296-
let currNode = this.getOrCreateIdentifier(n.identifier);
307+
let currNode = this.getOrCreateIdentifier(n.identifier, n.reactive);
297308
if (n.path.length === 0) {
298309
return currNode;
299310
}
@@ -315,10 +326,11 @@ function getMaybeNonNullInInstruction(
315326
instr: InstructionValue,
316327
context: CollectHoistablePropertyLoadsContext,
317328
): PropertyPathNode | null {
318-
let path = null;
329+
let path: ReactiveScopeDependency | null = null;
319330
if (instr.kind === 'PropertyLoad') {
320331
path = context.temporaries.get(instr.object.identifier.id) ?? {
321332
identifier: instr.object.identifier,
333+
reactive: instr.object.reactive,
322334
path: [],
323335
};
324336
} else if (instr.kind === 'Destructure') {
@@ -381,7 +393,7 @@ function collectNonNullsInBlocks(
381393
) {
382394
const identifier = fn.params[0].identifier;
383395
knownNonNullIdentifiers.add(
384-
context.registry.getOrCreateIdentifier(identifier),
396+
context.registry.getOrCreateIdentifier(identifier, true),
385397
);
386398
}
387399
const nodes = new Map<
@@ -616,9 +628,11 @@ function reduceMaybeOptionalChains(
616628
changed = false;
617629

618630
for (const original of optionalChainNodes) {
619-
let {identifier, path: origPath} = original.fullPath;
620-
let currNode: PropertyPathNode =
621-
registry.getOrCreateIdentifier(identifier);
631+
let {identifier, path: origPath, reactive} = original.fullPath;
632+
let currNode: PropertyPathNode = registry.getOrCreateIdentifier(
633+
identifier,
634+
reactive,
635+
);
622636
for (let i = 0; i < origPath.length; i++) {
623637
const entry = origPath[i];
624638
// If the base is known to be non-null, replace with a non-optional load

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ function traverseOptionalBlock(
290290
);
291291
baseObject = {
292292
identifier: maybeTest.instructions[0].value.place.identifier,
293+
reactive: maybeTest.instructions[0].value.place.reactive,
293294
path,
294295
};
295296
test = maybeTest.terminal;
@@ -391,6 +392,7 @@ function traverseOptionalBlock(
391392
);
392393
const load = {
393394
identifier: baseObject.identifier,
395+
reactive: baseObject.reactive,
394396
path: [
395397
...baseObject.path,
396398
{

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

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ export class ReactiveScopeDependencyTreeHIR {
2525
* `identifier.path`, or `identifier?.path` is in this map, it is safe to
2626
* evaluate (non-optional) PropertyLoads from.
2727
*/
28-
#hoistableObjects: Map<Identifier, HoistableNode> = new Map();
29-
#deps: Map<Identifier, DependencyNode> = new Map();
28+
#hoistableObjects: Map<Identifier, HoistableNode & {reactive: boolean}> =
29+
new Map();
30+
#deps: Map<Identifier, DependencyNode & {reactive: boolean}> = new Map();
3031

3132
/**
3233
* @param hoistableObjects a set of paths from which we can safely evaluate
@@ -35,9 +36,10 @@ export class ReactiveScopeDependencyTreeHIR {
3536
* duplicates when traversing the CFG.
3637
*/
3738
constructor(hoistableObjects: Iterable<ReactiveScopeDependency>) {
38-
for (const {path, identifier} of hoistableObjects) {
39+
for (const {path, identifier, reactive} of hoistableObjects) {
3940
let currNode = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot(
4041
identifier,
42+
reactive,
4143
this.#hoistableObjects,
4244
path.length > 0 && path[0].optional ? 'Optional' : 'NonNull',
4345
);
@@ -70,7 +72,8 @@ export class ReactiveScopeDependencyTreeHIR {
7072

7173
static #getOrCreateRoot<T extends string>(
7274
identifier: Identifier,
73-
roots: Map<Identifier, TreeNode<T>>,
75+
reactive: boolean,
76+
roots: Map<Identifier, TreeNode<T> & {reactive: boolean}>,
7477
defaultAccessType: T,
7578
): TreeNode<T> {
7679
// roots can always be accessed unconditionally in JS
@@ -79,9 +82,16 @@ export class ReactiveScopeDependencyTreeHIR {
7982
if (rootNode === undefined) {
8083
rootNode = {
8184
properties: new Map(),
85+
reactive,
8286
accessType: defaultAccessType,
8387
};
8488
roots.set(identifier, rootNode);
89+
} else {
90+
CompilerError.invariant(reactive === rootNode.reactive, {
91+
reason: '[DeriveMinimalDependenciesHIR] Conflicting reactive root flag',
92+
description: `Identifier ${printIdentifier(identifier)}`,
93+
loc: GeneratedSource,
94+
});
8595
}
8696
return rootNode;
8797
}
@@ -92,9 +102,10 @@ export class ReactiveScopeDependencyTreeHIR {
92102
* safe-to-evaluate subpath
93103
*/
94104
addDependency(dep: ReactiveScopeDependency): void {
95-
const {identifier, path} = dep;
105+
const {identifier, reactive, path} = dep;
96106
let depCursor = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot(
97107
identifier,
108+
reactive,
98109
this.#deps,
99110
PropertyAccessType.UnconditionalAccess,
100111
);
@@ -172,7 +183,13 @@ export class ReactiveScopeDependencyTreeHIR {
172183
deriveMinimalDependencies(): Set<ReactiveScopeDependency> {
173184
const results = new Set<ReactiveScopeDependency>();
174185
for (const [rootId, rootNode] of this.#deps.entries()) {
175-
collectMinimalDependenciesInSubtree(rootNode, rootId, [], results);
186+
collectMinimalDependenciesInSubtree(
187+
rootNode,
188+
rootNode.reactive,
189+
rootId,
190+
[],
191+
results,
192+
);
176193
}
177194

178195
return results;
@@ -294,25 +311,24 @@ type HoistableNode = TreeNode<'Optional' | 'NonNull'>;
294311
type DependencyNode = TreeNode<PropertyAccessType>;
295312

296313
/**
297-
* TODO: this is directly pasted from DeriveMinimalDependencies. Since we no
298-
* longer have conditionally accessed nodes, we can simplify
299-
*
300314
* Recursively calculates minimal dependencies in a subtree.
301315
* @param node DependencyNode representing a dependency subtree.
302316
* @returns a minimal list of dependencies in this subtree.
303317
*/
304318
function collectMinimalDependenciesInSubtree(
305319
node: DependencyNode,
320+
reactive: boolean,
306321
rootIdentifier: Identifier,
307322
path: Array<DependencyPathEntry>,
308323
results: Set<ReactiveScopeDependency>,
309324
): void {
310325
if (isDependency(node.accessType)) {
311-
results.add({identifier: rootIdentifier, path});
326+
results.add({identifier: rootIdentifier, reactive, path});
312327
} else {
313328
for (const [childName, childNode] of node.properties) {
314329
collectMinimalDependenciesInSubtree(
315330
childNode,
331+
reactive,
316332
rootIdentifier,
317333
[
318334
...path,

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1568,6 +1568,18 @@ export type DependencyPathEntry = {
15681568
export type DependencyPath = Array<DependencyPathEntry>;
15691569
export type ReactiveScopeDependency = {
15701570
identifier: Identifier;
1571+
/**
1572+
* Reflects whether the base identifier is reactive. Note that some reactive
1573+
* objects may have non-reactive properties, but we do not currently track
1574+
* this.
1575+
*
1576+
* ```js
1577+
* // Technically, result[0] is reactive and result[1] is not.
1578+
* // Currently, both dependencies would be marked as reactive.
1579+
* const result = useState();
1580+
* ```
1581+
*/
1582+
reactive: boolean;
15711583
path: DependencyPath;
15721584
};
15731585

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,7 @@ function collectTemporariesSidemapImpl(
316316
) {
317317
temporaries.set(lvalue.identifier.id, {
318318
identifier: value.place.identifier,
319+
reactive: value.place.reactive,
319320
path: [],
320321
});
321322
}
@@ -369,11 +370,13 @@ function getProperty(
369370
if (resolvedDependency == null) {
370371
property = {
371372
identifier: object.identifier,
373+
reactive: object.reactive,
372374
path: [{property: propertyName, optional}],
373375
};
374376
} else {
375377
property = {
376378
identifier: resolvedDependency.identifier,
379+
reactive: resolvedDependency.reactive,
377380
path: [...resolvedDependency.path, {property: propertyName, optional}],
378381
};
379382
}
@@ -532,6 +535,7 @@ export class DependencyCollectionContext {
532535
this.visitDependency(
533536
this.#temporaries.get(place.identifier.id) ?? {
534537
identifier: place.identifier,
538+
reactive: place.reactive,
535539
path: [],
536540
},
537541
);
@@ -596,6 +600,7 @@ export class DependencyCollectionContext {
596600
) {
597601
maybeDependency = {
598602
identifier: maybeDependency.identifier,
603+
reactive: maybeDependency.reactive,
599604
path: [],
600605
};
601606
}
@@ -617,7 +622,11 @@ export class DependencyCollectionContext {
617622
identifier =>
618623
identifier.declarationId === place.identifier.declarationId,
619624
) &&
620-
this.#checkValidDependency({identifier: place.identifier, path: []})
625+
this.#checkValidDependency({
626+
identifier: place.identifier,
627+
reactive: place.reactive,
628+
path: [],
629+
})
621630
) {
622631
currentScope.reassignments.add(place.identifier);
623632
}

compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
import {PostDominator} from '../HIR/Dominator';
2727
import {
2828
eachInstructionLValue,
29+
eachInstructionOperand,
2930
eachInstructionValueOperand,
3031
eachTerminalOperand,
3132
} from '../HIR/visitors';
@@ -292,7 +293,7 @@ export function inferReactivePlaces(fn: HIRFunction): void {
292293
let hasReactiveInput = false;
293294
/*
294295
* NOTE: we want to mark all operands as reactive or not, so we
295-
* avoid short-circuting here
296+
* avoid short-circuiting here
296297
*/
297298
for (const operand of eachInstructionValueOperand(value)) {
298299
const reactive = reactiveIdentifiers.isReactive(operand);
@@ -375,6 +376,41 @@ export function inferReactivePlaces(fn: HIRFunction): void {
375376
}
376377
}
377378
} while (reactiveIdentifiers.snapshot());
379+
380+
function propagateReactivityToInnerFunctions(
381+
fn: HIRFunction,
382+
isOutermost: boolean,
383+
): void {
384+
for (const [, block] of fn.body.blocks) {
385+
for (const instr of block.instructions) {
386+
if (!isOutermost) {
387+
for (const operand of eachInstructionOperand(instr)) {
388+
reactiveIdentifiers.isReactive(operand);
389+
}
390+
}
391+
if (
392+
instr.value.kind === 'ObjectMethod' ||
393+
instr.value.kind === 'FunctionExpression'
394+
) {
395+
propagateReactivityToInnerFunctions(
396+
instr.value.loweredFunc.func,
397+
false,
398+
);
399+
}
400+
}
401+
if (!isOutermost) {
402+
for (const operand of eachTerminalOperand(block.terminal)) {
403+
reactiveIdentifiers.isReactive(operand);
404+
}
405+
}
406+
}
407+
}
408+
409+
/**
410+
* Propagate reactivity for inner functions, as we eventually hoist and dedupe
411+
* dependency instructions for scopes.
412+
*/
413+
propagateReactivityToInnerFunctions(fn, true);
378414
}
379415

380416
/*

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,7 @@ function canMergeScopes(
456456
new Set(
457457
[...current.scope.declarations.values()].map(declaration => ({
458458
identifier: declaration.identifier,
459+
reactive: true,
459460
path: [],
460461
})),
461462
),

0 commit comments

Comments
 (0)