Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[compiler] Infer optional dependencies (behind feature flag) #30819

Merged
merged 10 commits into from
Aug 28, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,14 @@ const EnvironmentConfigSchema = z.object({

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

/**
* Enables inference of optional dependency chains. Without this flag
* a property chain such as `props?.items?.foo` will infer as a dep on
* just `props`. With this flag enabled, we'll infer that full path as
* the dependency.
*/
enableOptionalDependencies: z.boolean().default(false),

/*
* Enable validation of hooks to partially check that the component honors the rules of hooks.
* When disabled, the component is assumed to follow the rules (though the Babel plugin looks
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export function printTerminal(terminal: Terminal): Array<string> | string {
case 'branch': {
value = `[${terminal.id}] Branch (${printPlace(terminal.test)}) then:bb${
terminal.consequent
} else:bb${terminal.alternate}`;
} else:bb${terminal.alternate} fallthrough:bb${terminal.fallthrough}`;
break;
}
case 'logical': {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1446,9 +1446,19 @@ function codegenDependency(
dependency: ReactiveScopeDependency,
): t.Expression {
let object: t.Expression = convertIdentifier(dependency.identifier);
if (dependency.path !== null) {
if (dependency.path.length !== 0) {
const hasOptional = dependency.path.some(path => path.optional);
for (const path of dependency.path) {
object = t.memberExpression(object, t.identifier(path.property));
if (hasOptional) {
object = t.optionalMemberExpression(
object,
t.identifier(path.property),
false,
path.optional,
);
} else {
object = t.memberExpression(object, t.identifier(path.property));
}
}
}
return object;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,14 @@ export class ReactiveScopeDependencyTree {
const {path} = dep;
let currNode = this.#getOrCreateRoot(dep.identifier);

const accessType = inConditional
? PropertyAccessType.ConditionalAccess
: PropertyAccessType.UnconditionalAccess;

for (const item of path) {
// all properties read 'on the way' to a dependency are marked as 'access'
let currChild = getOrMakeProperty(currNode, item.property);
const accessType = inConditional
? PropertyAccessType.ConditionalAccess
: item.optional
? PropertyAccessType.OptionalAccess
: PropertyAccessType.UnconditionalAccess;
currChild.accessType = merge(currChild.accessType, accessType);
currNode = currChild;
}
Expand All @@ -77,18 +78,22 @@ export class ReactiveScopeDependencyTree {
*/
const depType = inConditional
? PropertyAccessType.ConditionalDependency
: PropertyAccessType.UnconditionalDependency;
: isOptional(currNode.accessType)
? PropertyAccessType.OptionalDependency
: PropertyAccessType.UnconditionalDependency;

currNode.accessType = merge(currNode.accessType, depType);
}

deriveMinimalDependencies(): Set<ReactiveScopeDependency> {
const results = new Set<ReactiveScopeDependency>();
for (const [rootId, rootNode] of this.#roots.entries()) {
const deps = deriveMinimalDependenciesInSubtree(rootNode);
const deps = deriveMinimalDependenciesInSubtree(rootNode, null);
CompilerError.invariant(
deps.every(
dep => dep.accessType === PropertyAccessType.UnconditionalDependency,
dep =>
dep.accessType === PropertyAccessType.UnconditionalDependency ||
dep.accessType == PropertyAccessType.OptionalDependency,
),
{
reason:
Expand Down Expand Up @@ -173,6 +178,27 @@ export class ReactiveScopeDependencyTree {
}
return res.flat().join('\n');
}

debug(): string {
const buf: Array<string> = [`tree() [`];
for (const [rootId, rootNode] of this.#roots) {
buf.push(`${printIdentifier(rootId)} (${rootNode.accessType}):`);
this.#debugImpl(buf, rootNode, 1);
}
buf.push(']');
return buf.length > 2 ? buf.join('\n') : buf.join('');
}

#debugImpl(
buf: Array<string>,
node: DependencyNode,
depth: number = 0,
): void {
for (const [property, childNode] of node.properties) {
buf.push(`${' '.repeat(depth)}.${property} (${childNode.accessType}):`);
this.#debugImpl(buf, childNode, depth + 1);
}
}
}

/*
Expand All @@ -196,8 +222,10 @@ export class ReactiveScopeDependencyTree {
*/
enum PropertyAccessType {
ConditionalAccess = 'ConditionalAccess',
OptionalAccess = 'OptionalAccess',
UnconditionalAccess = 'UnconditionalAccess',
ConditionalDependency = 'ConditionalDependency',
OptionalDependency = 'OptionalDependency',
UnconditionalDependency = 'UnconditionalDependency',
}

Expand All @@ -211,9 +239,16 @@ function isUnconditional(access: PropertyAccessType): boolean {
function isDependency(access: PropertyAccessType): boolean {
return (
access === PropertyAccessType.ConditionalDependency ||
access === PropertyAccessType.OptionalDependency ||
access === PropertyAccessType.UnconditionalDependency
);
}
function isOptional(access: PropertyAccessType): boolean {
return (
access === PropertyAccessType.OptionalAccess ||
access === PropertyAccessType.OptionalDependency
);
}

function merge(
access1: PropertyAccessType,
Expand All @@ -222,6 +257,7 @@ function merge(
const resultIsUnconditional =
isUnconditional(access1) || isUnconditional(access2);
const resultIsDependency = isDependency(access1) || isDependency(access2);
const resultIsOptional = isOptional(access1) || isOptional(access2);

/*
* Straightforward merge.
Expand All @@ -237,6 +273,12 @@ function merge(
} else {
return PropertyAccessType.UnconditionalAccess;
}
} else if (resultIsOptional) {
if (resultIsDependency) {
return PropertyAccessType.OptionalDependency;
} else {
return PropertyAccessType.OptionalAccess;
}
} else {
if (resultIsDependency) {
return PropertyAccessType.ConditionalDependency;
Expand All @@ -256,19 +298,34 @@ type ReduceResultNode = {
accessType: PropertyAccessType;
};

const promoteUncondResult = [
{
function promoteResult(
accessType: PropertyAccessType,
path: {property: string; optional: boolean} | null,
): Array<ReduceResultNode> {
const result: ReduceResultNode = {
relativePath: [],
accessType: PropertyAccessType.UnconditionalDependency,
},
];
accessType,
};
if (path !== null) {
result.relativePath.push(path);
}
return [result];
}

const promoteCondResult = [
{
relativePath: [],
accessType: PropertyAccessType.ConditionalDependency,
},
];
function prependPath(
results: Array<ReduceResultNode>,
path: {property: string; optional: boolean} | null,
): Array<ReduceResultNode> {
if (path === null) {
return results;
}
return results.map(result => {
return {
accessType: result.accessType,
relativePath: [path, ...result.relativePath],
};
});
}

/*
* Recursively calculates minimal dependencies in a subtree.
Expand All @@ -277,42 +334,76 @@ const promoteCondResult = [
*/
function deriveMinimalDependenciesInSubtree(
dep: DependencyNode,
property: string | null,
): Array<ReduceResultNode> {
const results: Array<ReduceResultNode> = [];
for (const [childName, childNode] of dep.properties) {
const childResult = deriveMinimalDependenciesInSubtree(childNode).map(
({relativePath, accessType}) => {
return {
relativePath: [
{property: childName, optional: false},
...relativePath,
],
accessType,
};
},
const childResult = deriveMinimalDependenciesInSubtree(
childNode,
childName,
);
results.push(...childResult);
}

switch (dep.accessType) {
case PropertyAccessType.UnconditionalDependency: {
return promoteUncondResult;
return promoteResult(
PropertyAccessType.UnconditionalDependency,
property !== null ? {property, optional: false} : null,
);
}
case PropertyAccessType.UnconditionalAccess: {
if (
results.every(
({accessType}) =>
accessType === PropertyAccessType.UnconditionalDependency,
accessType === PropertyAccessType.UnconditionalDependency ||
accessType === PropertyAccessType.OptionalDependency,
)
) {
// all children are unconditional dependencies, return them to preserve granularity
return results;
return prependPath(
results,
property !== null ? {property, optional: false} : null,
);
} else {
/*
* at least one child is accessed conditionally, so this node needs to be promoted to
* unconditional dependency
*/
return promoteUncondResult;
return promoteResult(
PropertyAccessType.UnconditionalDependency,
property !== null ? {property, optional: false} : null,
);
}
}
case PropertyAccessType.OptionalDependency: {
return promoteResult(
PropertyAccessType.OptionalDependency,
property !== null ? {property, optional: true} : null,
);
}
case PropertyAccessType.OptionalAccess: {
if (
results.every(
({accessType}) =>
accessType === PropertyAccessType.UnconditionalDependency ||
accessType === PropertyAccessType.OptionalDependency,
)
) {
// all children are unconditional dependencies, return them to preserve granularity
return prependPath(
results,
property !== null ? {property, optional: true} : null,
);
} else {
/*
* at least one child is accessed conditionally, so this node needs to be promoted to
* unconditional dependency
*/
return promoteResult(
PropertyAccessType.OptionalDependency,
property !== null ? {property, optional: true} : null,
);
}
}
case PropertyAccessType.ConditionalAccess:
Expand All @@ -328,13 +419,19 @@ function deriveMinimalDependenciesInSubtree(
* unconditional access.
* Truncate results of child nodes here, since we shouldn't access them anyways
*/
return promoteCondResult;
return promoteResult(
PropertyAccessType.ConditionalDependency,
property !== null ? {property, optional: true} : null,
);
} else {
/*
* at least one child is accessed unconditionally, so this node can be promoted to
* unconditional dependency
*/
return promoteUncondResult;
return promoteResult(
PropertyAccessType.UnconditionalDependency,
property !== null ? {property, optional: true} : null,
);
}
}
default: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import {CompilerError} from '..';
import {
DeclarationId,
DependencyPath,
InstructionId,
InstructionKind,
Place,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export function printDependency(dependency: ReactiveScopeDependency): string {
const identifier =
printIdentifier(dependency.identifier) +
printType(dependency.identifier.type);
return `${identifier}${dependency.path.map(token => `.${token.property}`).join('')}`;
return `${identifier}${dependency.path.map(token => `${token.optional ? '?.' : '.'}${token.property}`).join('')}`;
}

export function printReactiveInstructions(
Expand Down
Loading