Compare commits

...

1 Commits

Author SHA1 Message Date
Mofei Zhang
90131ceca5 [compiler] Add reactive flag on scope dependencies
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();
```
2025-05-22 15:44:36 -04:00
7 changed files with 109 additions and 19 deletions

View File

@@ -241,7 +241,10 @@ type PropertyPathNode =
class PropertyPathRegistry {
roots: Map<IdentifierId, RootNode> = new Map();
getOrCreateIdentifier(identifier: Identifier): PropertyPathNode {
getOrCreateIdentifier(
identifier: Identifier,
reactive: boolean,
): PropertyPathNode {
/**
* Reads from a statically scoped variable are always safe in JS,
* with the exception of TDZ (not addressed by this pass).
@@ -255,12 +258,19 @@ class PropertyPathRegistry {
optionalProperties: new Map(),
fullPath: {
identifier,
reactive,
path: [],
},
hasOptional: false,
parent: null,
};
this.roots.set(identifier.id, rootNode);
} else {
CompilerError.invariant(reactive === rootNode.fullPath.reactive, {
reason:
'[HoistablePropertyLoads] Found inconsistencies in `reactive` flag when deduping identifier reads within the same scope',
loc: identifier.loc,
});
}
return rootNode;
}
@@ -278,6 +288,7 @@ class PropertyPathRegistry {
parent: parent,
fullPath: {
identifier: parent.fullPath.identifier,
reactive: parent.fullPath.reactive,
path: parent.fullPath.path.concat(entry),
},
hasOptional: parent.hasOptional || entry.optional,
@@ -293,7 +304,7 @@ class PropertyPathRegistry {
* so all subpaths of a PropertyLoad should already exist
* (e.g. a.b is added before a.b.c),
*/
let currNode = this.getOrCreateIdentifier(n.identifier);
let currNode = this.getOrCreateIdentifier(n.identifier, n.reactive);
if (n.path.length === 0) {
return currNode;
}
@@ -315,10 +326,11 @@ function getMaybeNonNullInInstruction(
instr: InstructionValue,
context: CollectHoistablePropertyLoadsContext,
): PropertyPathNode | null {
let path = null;
let path: ReactiveScopeDependency | null = null;
if (instr.kind === 'PropertyLoad') {
path = context.temporaries.get(instr.object.identifier.id) ?? {
identifier: instr.object.identifier,
reactive: instr.object.reactive,
path: [],
};
} else if (instr.kind === 'Destructure') {
@@ -381,7 +393,7 @@ function collectNonNullsInBlocks(
) {
const identifier = fn.params[0].identifier;
knownNonNullIdentifiers.add(
context.registry.getOrCreateIdentifier(identifier),
context.registry.getOrCreateIdentifier(identifier, true),
);
}
const nodes = new Map<
@@ -616,9 +628,11 @@ function reduceMaybeOptionalChains(
changed = false;
for (const original of optionalChainNodes) {
let {identifier, path: origPath} = original.fullPath;
let currNode: PropertyPathNode =
registry.getOrCreateIdentifier(identifier);
let {identifier, path: origPath, reactive} = original.fullPath;
let currNode: PropertyPathNode = registry.getOrCreateIdentifier(
identifier,
reactive,
);
for (let i = 0; i < origPath.length; i++) {
const entry = origPath[i];
// If the base is known to be non-null, replace with a non-optional load

View File

@@ -290,6 +290,7 @@ function traverseOptionalBlock(
);
baseObject = {
identifier: maybeTest.instructions[0].value.place.identifier,
reactive: maybeTest.instructions[0].value.place.reactive,
path,
};
test = maybeTest.terminal;
@@ -391,6 +392,7 @@ function traverseOptionalBlock(
);
const load = {
identifier: baseObject.identifier,
reactive: baseObject.reactive,
path: [
...baseObject.path,
{

View File

@@ -25,8 +25,9 @@ export class ReactiveScopeDependencyTreeHIR {
* `identifier.path`, or `identifier?.path` is in this map, it is safe to
* evaluate (non-optional) PropertyLoads from.
*/
#hoistableObjects: Map<Identifier, HoistableNode> = new Map();
#deps: Map<Identifier, DependencyNode> = new Map();
#hoistableObjects: Map<Identifier, HoistableNode & {reactive: boolean}> =
new Map();
#deps: Map<Identifier, DependencyNode & {reactive: boolean}> = new Map();
/**
* @param hoistableObjects a set of paths from which we can safely evaluate
@@ -35,9 +36,10 @@ export class ReactiveScopeDependencyTreeHIR {
* duplicates when traversing the CFG.
*/
constructor(hoistableObjects: Iterable<ReactiveScopeDependency>) {
for (const {path, identifier} of hoistableObjects) {
for (const {path, identifier, reactive} of hoistableObjects) {
let currNode = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot(
identifier,
reactive,
this.#hoistableObjects,
path.length > 0 && path[0].optional ? 'Optional' : 'NonNull',
);
@@ -70,7 +72,8 @@ export class ReactiveScopeDependencyTreeHIR {
static #getOrCreateRoot<T extends string>(
identifier: Identifier,
roots: Map<Identifier, TreeNode<T>>,
reactive: boolean,
roots: Map<Identifier, TreeNode<T> & {reactive: boolean}>,
defaultAccessType: T,
): TreeNode<T> {
// roots can always be accessed unconditionally in JS
@@ -79,9 +82,16 @@ export class ReactiveScopeDependencyTreeHIR {
if (rootNode === undefined) {
rootNode = {
properties: new Map(),
reactive,
accessType: defaultAccessType,
};
roots.set(identifier, rootNode);
} else {
CompilerError.invariant(reactive === rootNode.reactive, {
reason: '[DeriveMinimalDependenciesHIR] Conflicting reactive root flag',
description: `Identifier ${printIdentifier(identifier)}`,
loc: GeneratedSource,
});
}
return rootNode;
}
@@ -92,9 +102,10 @@ export class ReactiveScopeDependencyTreeHIR {
* safe-to-evaluate subpath
*/
addDependency(dep: ReactiveScopeDependency): void {
const {identifier, path} = dep;
const {identifier, reactive, path} = dep;
let depCursor = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot(
identifier,
reactive,
this.#deps,
PropertyAccessType.UnconditionalAccess,
);
@@ -172,7 +183,13 @@ export class ReactiveScopeDependencyTreeHIR {
deriveMinimalDependencies(): Set<ReactiveScopeDependency> {
const results = new Set<ReactiveScopeDependency>();
for (const [rootId, rootNode] of this.#deps.entries()) {
collectMinimalDependenciesInSubtree(rootNode, rootId, [], results);
collectMinimalDependenciesInSubtree(
rootNode,
rootNode.reactive,
rootId,
[],
results,
);
}
return results;
@@ -294,25 +311,24 @@ type HoistableNode = TreeNode<'Optional' | 'NonNull'>;
type DependencyNode = TreeNode<PropertyAccessType>;
/**
* TODO: this is directly pasted from DeriveMinimalDependencies. Since we no
* longer have conditionally accessed nodes, we can simplify
*
* Recursively calculates minimal dependencies in a subtree.
* @param node DependencyNode representing a dependency subtree.
* @returns a minimal list of dependencies in this subtree.
*/
function collectMinimalDependenciesInSubtree(
node: DependencyNode,
reactive: boolean,
rootIdentifier: Identifier,
path: Array<DependencyPathEntry>,
results: Set<ReactiveScopeDependency>,
): void {
if (isDependency(node.accessType)) {
results.add({identifier: rootIdentifier, path});
results.add({identifier: rootIdentifier, reactive, path});
} else {
for (const [childName, childNode] of node.properties) {
collectMinimalDependenciesInSubtree(
childNode,
reactive,
rootIdentifier,
[
...path,

View File

@@ -1568,6 +1568,18 @@ export type DependencyPathEntry = {
export type DependencyPath = Array<DependencyPathEntry>;
export type ReactiveScopeDependency = {
identifier: Identifier;
/**
* Reflects whether the base identifier is reactive. Note that some reactive
* objects may have non-reactive properties, but we do not currently track
* this.
*
* ```js
* // Technically, result[0] is reactive and result[1] is not.
* // Currently, both dependencies would be marked as reactive.
* const result = useState();
* ```
*/
reactive: boolean;
path: DependencyPath;
};

View File

@@ -316,6 +316,7 @@ function collectTemporariesSidemapImpl(
) {
temporaries.set(lvalue.identifier.id, {
identifier: value.place.identifier,
reactive: value.place.reactive,
path: [],
});
}
@@ -369,11 +370,13 @@ function getProperty(
if (resolvedDependency == null) {
property = {
identifier: object.identifier,
reactive: object.reactive,
path: [{property: propertyName, optional}],
};
} else {
property = {
identifier: resolvedDependency.identifier,
reactive: resolvedDependency.reactive,
path: [...resolvedDependency.path, {property: propertyName, optional}],
};
}
@@ -532,6 +535,7 @@ export class DependencyCollectionContext {
this.visitDependency(
this.#temporaries.get(place.identifier.id) ?? {
identifier: place.identifier,
reactive: place.reactive,
path: [],
},
);
@@ -596,6 +600,7 @@ export class DependencyCollectionContext {
) {
maybeDependency = {
identifier: maybeDependency.identifier,
reactive: maybeDependency.reactive,
path: [],
};
}
@@ -617,7 +622,11 @@ export class DependencyCollectionContext {
identifier =>
identifier.declarationId === place.identifier.declarationId,
) &&
this.#checkValidDependency({identifier: place.identifier, path: []})
this.#checkValidDependency({
identifier: place.identifier,
reactive: place.reactive,
path: [],
})
) {
currentScope.reassignments.add(place.identifier);
}

View File

@@ -26,6 +26,7 @@ import {
import {PostDominator} from '../HIR/Dominator';
import {
eachInstructionLValue,
eachInstructionOperand,
eachInstructionValueOperand,
eachTerminalOperand,
} from '../HIR/visitors';
@@ -292,7 +293,7 @@ export function inferReactivePlaces(fn: HIRFunction): void {
let hasReactiveInput = false;
/*
* NOTE: we want to mark all operands as reactive or not, so we
* avoid short-circuting here
* avoid short-circuiting here
*/
for (const operand of eachInstructionValueOperand(value)) {
const reactive = reactiveIdentifiers.isReactive(operand);
@@ -375,6 +376,41 @@ export function inferReactivePlaces(fn: HIRFunction): void {
}
}
} while (reactiveIdentifiers.snapshot());
function propagateReactivityToInnerFunctions(
fn: HIRFunction,
isOutermost: boolean,
): void {
for (const [, block] of fn.body.blocks) {
for (const instr of block.instructions) {
if (!isOutermost) {
for (const operand of eachInstructionOperand(instr)) {
reactiveIdentifiers.isReactive(operand);
}
}
if (
instr.value.kind === 'ObjectMethod' ||
instr.value.kind === 'FunctionExpression'
) {
propagateReactivityToInnerFunctions(
instr.value.loweredFunc.func,
false,
);
}
}
if (!isOutermost) {
for (const operand of eachTerminalOperand(block.terminal)) {
reactiveIdentifiers.isReactive(operand);
}
}
}
}
/**
* Propagate reactivity for inner functions, as we eventually hoist and dedupe
* dependency instructions for scopes.
*/
propagateReactivityToInnerFunctions(fn, true);
}
/*

View File

@@ -456,6 +456,7 @@ function canMergeScopes(
new Set(
[...current.scope.declarations.values()].map(declaration => ({
identifier: declaration.identifier,
reactive: true,
path: [],
})),
),