@samitouri / QOS-React / commits / abf9fd559d

[compiler] Add reactive flag on scope dependencies (#33325)

When collecting scope dependencies, mark each dependency with `reactive: true | false`. This prepares for later PRs https://github.com/facebook/react/pull/33326 and https://github.com/facebook/react/pull/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(); ``` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33325). * #33326 * __->__ #33325 * #32286

mofeiZ committed May 22, 2025 at 16:14 UTC abf9fd559d584278c1c5f5464e35290651cf82bc
7 files changed +109 -19
compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts
+21 -7
@@ -241,7 +241,10 @@ type PropertyPathNode =
241 class PropertyPathRegistry {
242 roots: Map<IdentifierId, RootNode> = new Map();
243
244 - getOrCreateIdentifier(identifier: Identifier): PropertyPathNode {
244 + getOrCreateIdentifier(
245 + identifier: Identifier,
246 + reactive: boolean,
247 + ): PropertyPathNode {
248 /**
249 * Reads from a statically scoped variable are always safe in JS,
250 * with the exception of TDZ (not addressed by this pass).
@@ -255,12 +258,19 @@ class PropertyPathRegistry {
258 optionalProperties: new Map(),
259 fullPath: {
260 identifier,
261 + reactive,
262 path: [],
263 },
264 hasOptional: false,
265 parent: null,
266 };
267 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 + });
274 }
275 return rootNode;
276 }
@@ -278,6 +288,7 @@ class PropertyPathRegistry {
288 parent: parent,
289 fullPath: {
290 identifier: parent.fullPath.identifier,
291 + reactive: parent.fullPath.reactive,
292 path: parent.fullPath.path.concat(entry),
293 },
294 hasOptional: parent.hasOptional || entry.optional,
@@ -293,7 +304,7 @@ class PropertyPathRegistry {
304 * so all subpaths of a PropertyLoad should already exist
305 * (e.g. a.b is added before a.b.c),
306 */
296 - let currNode = this.getOrCreateIdentifier(n.identifier);
307 + let currNode = this.getOrCreateIdentifier(n.identifier, n.reactive);
308 if (n.path.length === 0) {
309 return currNode;
310 }
@@ -315,10 +326,11 @@ function getMaybeNonNullInInstruction(
326 instr: InstructionValue,
327 context: CollectHoistablePropertyLoadsContext,
328 ): PropertyPathNode | null {
318 - let path = null;
329 + let path: ReactiveScopeDependency | null = null;
330 if (instr.kind === 'PropertyLoad') {
331 path = context.temporaries.get(instr.object.identifier.id) ?? {
332 identifier: instr.object.identifier,
333 + reactive: instr.object.reactive,
334 path: [],
335 };
336 } else if (instr.kind === 'Destructure') {
@@ -381,7 +393,7 @@ function collectNonNullsInBlocks(
393 ) {
394 const identifier = fn.params[0].identifier;
395 knownNonNullIdentifiers.add(
384 - context.registry.getOrCreateIdentifier(identifier),
396 + context.registry.getOrCreateIdentifier(identifier, true),
397 );
398 }
399 const nodes = new Map<
@@ -616,9 +628,11 @@ function reduceMaybeOptionalChains(
628 changed = false;
629
630 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 + );
636 for (let i = 0; i < origPath.length; i++) {
637 const entry = origPath[i];
638 // 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
+2
@@ -290,6 +290,7 @@ function traverseOptionalBlock(
290 );
291 baseObject = {
292 identifier: maybeTest.instructions[0].value.place.identifier,
293 + reactive: maybeTest.instructions[0].value.place.reactive,
294 path,
295 };
296 test = maybeTest.terminal;
@@ -391,6 +392,7 @@ function traverseOptionalBlock(
392 );
393 const load = {
394 identifier: baseObject.identifier,
395 + reactive: baseObject.reactive,
396 path: [
397 ...baseObject.path,
398 {
compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts
+26 -10
@@ -25,8 +25,9 @@ export class ReactiveScopeDependencyTreeHIR {
25 * `identifier.path`, or `identifier?.path` is in this map, it is safe to
26 * evaluate (non-optional) PropertyLoads from.
27 */
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();
31
32 /**
33 * @param hoistableObjects a set of paths from which we can safely evaluate
@@ -35,9 +36,10 @@ export class ReactiveScopeDependencyTreeHIR {
36 * duplicates when traversing the CFG.
37 */
38 constructor(hoistableObjects: Iterable<ReactiveScopeDependency>) {
38 - for (const {path, identifier} of hoistableObjects) {
39 + for (const {path, identifier, reactive} of hoistableObjects) {
40 let currNode = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot(
41 identifier,
42 + reactive,
43 this.#hoistableObjects,
44 path.length > 0 && path[0].optional ? 'Optional' : 'NonNull',
45 );
@@ -70,7 +72,8 @@ export class ReactiveScopeDependencyTreeHIR {
72
73 static #getOrCreateRoot<T extends string>(
74 identifier: Identifier,
73 - roots: Map<Identifier, TreeNode<T>>,
75 + reactive: boolean,
76 + roots: Map<Identifier, TreeNode<T> & {reactive: boolean}>,
77 defaultAccessType: T,
78 ): TreeNode<T> {
79 // roots can always be accessed unconditionally in JS
@@ -79,9 +82,16 @@ export class ReactiveScopeDependencyTreeHIR {
82 if (rootNode === undefined) {
83 rootNode = {
84 properties: new Map(),
85 + reactive,
86 accessType: defaultAccessType,
87 };
88 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 + });
95 }
96 return rootNode;
97 }
@@ -92,9 +102,10 @@ export class ReactiveScopeDependencyTreeHIR {
102 * safe-to-evaluate subpath
103 */
104 addDependency(dep: ReactiveScopeDependency): void {
95 - const {identifier, path} = dep;
105 + const {identifier, reactive, path} = dep;
106 let depCursor = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot(
107 identifier,
108 + reactive,
109 this.#deps,
110 PropertyAccessType.UnconditionalAccess,
111 );
@@ -172,7 +183,13 @@ export class ReactiveScopeDependencyTreeHIR {
183 deriveMinimalDependencies(): Set<ReactiveScopeDependency> {
184 const results = new Set<ReactiveScopeDependency>();
185 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 + );
193 }
194
195 return results;
@@ -294,25 +311,24 @@ type HoistableNode = TreeNode<'Optional' | 'NonNull'>;
311 type DependencyNode = TreeNode<PropertyAccessType>;
312
313 /**
297 - * TODO: this is directly pasted from DeriveMinimalDependencies. Since we no
298 - * longer have conditionally accessed nodes, we can simplify
299 - *
314 * Recursively calculates minimal dependencies in a subtree.
315 * @param node DependencyNode representing a dependency subtree.
316 * @returns a minimal list of dependencies in this subtree.
317 */
318 function collectMinimalDependenciesInSubtree(
319 node: DependencyNode,
320 + reactive: boolean,
321 rootIdentifier: Identifier,
322 path: Array<DependencyPathEntry>,
323 results: Set<ReactiveScopeDependency>,
324 ): void {
325 if (isDependency(node.accessType)) {
311 - results.add({identifier: rootIdentifier, path});
326 + results.add({identifier: rootIdentifier, reactive, path});
327 } else {
328 for (const [childName, childNode] of node.properties) {
329 collectMinimalDependenciesInSubtree(
330 childNode,
331 + reactive,
332 rootIdentifier,
333 [
334 ...path,
compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
+12
@@ -1568,6 +1568,18 @@ export type DependencyPathEntry = {
1568 export type DependencyPath = Array<DependencyPathEntry>;
1569 export type ReactiveScopeDependency = {
1570 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;
1583 path: DependencyPath;
1584 };
1585
compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts
+10 -1
@@ -316,6 +316,7 @@ function collectTemporariesSidemapImpl(
316 ) {
317 temporaries.set(lvalue.identifier.id, {
318 identifier: value.place.identifier,
319 + reactive: value.place.reactive,
320 path: [],
321 });
322 }
@@ -369,11 +370,13 @@ function getProperty(
370 if (resolvedDependency == null) {
371 property = {
372 identifier: object.identifier,
373 + reactive: object.reactive,
374 path: [{property: propertyName, optional}],
375 };
376 } else {
377 property = {
378 identifier: resolvedDependency.identifier,
379 + reactive: resolvedDependency.reactive,
380 path: [...resolvedDependency.path, {property: propertyName, optional}],
381 };
382 }
@@ -532,6 +535,7 @@ export class DependencyCollectionContext {
535 this.visitDependency(
536 this.#temporaries.get(place.identifier.id) ?? {
537 identifier: place.identifier,
538 + reactive: place.reactive,
539 path: [],
540 },
541 );
@@ -596,6 +600,7 @@ export class DependencyCollectionContext {
600 ) {
601 maybeDependency = {
602 identifier: maybeDependency.identifier,
603 + reactive: maybeDependency.reactive,
604 path: [],
605 };
606 }
@@ -617,7 +622,11 @@ export class DependencyCollectionContext {
622 identifier =>
623 identifier.declarationId === place.identifier.declarationId,
624 ) &&
620 - this.#checkValidDependency({identifier: place.identifier, path: []})
625 + this.#checkValidDependency({
626 + identifier: place.identifier,
627 + reactive: place.reactive,
628 + path: [],
629 + })
630 ) {
631 currentScope.reassignments.add(place.identifier);
632 }
compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts
+37 -1
@@ -26,6 +26,7 @@ import {
26 import {PostDominator} from '../HIR/Dominator';
27 import {
28 eachInstructionLValue,
29 + eachInstructionOperand,
30 eachInstructionValueOperand,
31 eachTerminalOperand,
32 } from '../HIR/visitors';
@@ -292,7 +293,7 @@ export function inferReactivePlaces(fn: HIRFunction): void {
293 let hasReactiveInput = false;
294 /*
295 * NOTE: we want to mark all operands as reactive or not, so we
295 - * avoid short-circuting here
296 + * avoid short-circuiting here
297 */
298 for (const operand of eachInstructionValueOperand(value)) {
299 const reactive = reactiveIdentifiers.isReactive(operand);
@@ -375,6 +376,41 @@ export function inferReactivePlaces(fn: HIRFunction): void {
376 }
377 }
378 } 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);
414 }
415
416 /*
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts
+1
@@ -456,6 +456,7 @@ function canMergeScopes(
456 new Set(
457 [...current.scope.declarations.values()].map(declaration => ({
458 identifier: declaration.identifier,
459 + reactive: true,
460 path: [],
461 })),
462 ),