fix(compiler-cli): account for more expression types when determining HMR dependencies (#59323)

During the HMR dependency analysis we need to check if an identifier is top-level or not. We do this by looking at each identifier and its parent, however we didn't account for some cases. These changes expand our logic to cover more of the common node types.

Related to https://github.com/angular/angular/issues/59310#issuecomment-2563963501.

PR Close #59323
This commit is contained in:
Kristiyan Kostadinov 2024-12-28 10:01:37 +02:00 committed by Jessica Janiuk
parent ceadd28ea1
commit ce3b6641fb
2 changed files with 67 additions and 4 deletions

View file

@ -209,8 +209,24 @@ class PotentialTopLevelReadsVisitor extends o.RecursiveAstVisitor {
return parent.expression === node || parent.arguments.includes(node);
}
// Identifier used in a property read is only top-level if it's the expression.
if (ts.isPropertyAccessExpression(parent)) {
// Identifier used in a nested expression is only top-level if it's the actual expression.
if (
ts.isPropertyAccessExpression(parent) ||
ts.isComputedPropertyName(parent) ||
ts.isTemplateSpan(parent) ||
ts.isSpreadAssignment(parent) ||
ts.isSpreadElement(parent) ||
ts.isAwaitExpression(parent) ||
ts.isNonNullExpression(parent) ||
ts.isIfStatement(parent) ||
ts.isDoStatement(parent) ||
ts.isWhileStatement(parent) ||
ts.isForInStatement(parent) ||
ts.isForOfStatement(parent) ||
ts.isSwitchStatement(parent) ||
ts.isCaseClause(parent) ||
ts.isThrowStatement(parent)
) {
return parent.expression === node;
}
@ -224,11 +240,31 @@ class PotentialTopLevelReadsVisitor extends o.RecursiveAstVisitor {
return parent.initializer === node;
}
// Identifier in a class is only top level if it's the name.
if (ts.isClassDeclaration(parent)) {
// Identifier in a declaration is only top level if it's the name.
// In shorthand assignments the name is also the value.
if (
ts.isClassDeclaration(parent) ||
ts.isFunctionDeclaration(parent) ||
ts.isVariableDeclaration(parent) ||
ts.isShorthandPropertyAssignment(parent)
) {
return parent.name === node;
}
if (ts.isElementAccessExpression(parent)) {
return parent.expression === node || parent.argumentExpression === node;
}
if (ts.isBinaryExpression(parent)) {
return parent.left === node || parent.right === node;
}
// It's unlikely that we'll run into imports/exports in this use case.
// We handle them since it's simple and for completeness' sake.
if (ts.isImportSpecifier(parent) || ts.isExportSpecifier(parent)) {
return (parent.propertyName || parent.name) === node;
}
// Otherwise it's not top-level.
return false;
}

View file

@ -349,5 +349,32 @@ runInEachFileSystem(() => {
const hmrContents = env.driveHmr('test.ts', 'Foo');
expect(hmrContents).toBe(null);
});
it('should capture shorthand property assignment dependencies', () => {
enableHmr();
env.write(
'test.ts',
`
import {Component} from '@angular/core';
const providers: any[] = [];
@Component({template: '', providers})
export class Cmp {}
`,
);
env.driveMain();
const jsContents = env.getContents('test.js');
const hmrContents = env.driveHmr('test.ts', 'Cmp');
expect(jsContents).toContain(
'ɵɵreplaceMetadata(Cmp, m.default, [i0], [providers, Component]));',
);
expect(hmrContents).toContain(
'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, providers, Component) {',
);
});
});
});