Skip to content

refactor(compiler-cli): Fix regressions caused by ts.typechecker removal #68073

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.

Sign up for GitHub

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

Merged
atscott merged 1 commit into from
Apr 8, 2026
Merged

refactor(compiler-cli): Fix regressions caused by ts.typechecker removal #68073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/src/comments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,39 @@ export function hasExpressionIdentifier(
return false;
}
const commentText = sourceFile.text.substring(pos + 2, end - 2);
return commentText === `${CommentTriviaType.EXPRESSION_TYPE_IDENTIFIER}:${identifier}`;
const prefix = `${CommentTriviaType.EXPRESSION_TYPE_IDENTIFIER}:${identifier}`;
return commentText === prefix || commentText.startsWith(prefix + ':');
}) || false
);
}

export function readDirectiveIdFromComment(
sourceFile: ts.SourceFile,
node: ts.Node,
): number | null {
let id: number | null = null;
ts.forEachTrailingCommentRange(sourceFile.text, node.getEnd(), (pos, end, kind) => {
if (kind !== ts.SyntaxKind.MultiLineCommentTrivia) {
return;
}
const commentText = sourceFile.text.substring(pos + 2, end - 2);
const prefix = `${CommentTriviaType.EXPRESSION_TYPE_IDENTIFIER}:${ExpressionIdentifier.DIRECTIVE}:`;
const hostPrefix = `${CommentTriviaType.EXPRESSION_TYPE_IDENTIFIER}:${ExpressionIdentifier.HOST_DIRECTIVE}:`;

let matchedPrefix: string | null = null;
if (commentText.startsWith(prefix)) {
matchedPrefix = prefix;
} else if (commentText.startsWith(hostPrefix)) {
matchedPrefix = hostPrefix;
}

if (matchedPrefix !== null) {
const idStr = commentText.substring(matchedPrefix.length);
const parsed = parseInt(idStr, 10);
if (!isNaN(parsed)) {
id = parsed;
}
}
});
return id;
}
4 changes: 2 additions & 2 deletions packages/compiler-cli/src/ngtsc/typecheck/src/ops/codegen.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ export class TcbExpr {
* Tags the expression with an identifier.
* @param identifier Identifier to apply to the expression.
*/
addExpressionIdentifier(identifier: ExpressionIdentifier): this {
this.identifierComment = `${CommentTriviaType.EXPRESSION_TYPE_IDENTIFIER}:${identifier}`;
addExpressionIdentifier(identifier: ExpressionIdentifier, id?: number): this {
this.identifierComment = `${CommentTriviaType.EXPRESSION_TYPE_IDENTIFIER}:${identifier}${id !== undefined ? `:${id}` : ''}`;
return this;
}

Expand Down
3 changes: 2 additions & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/src/ops/directive_constructor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export class TcbDirectiveCtorOp extends TcbOp {
private node: DirectiveOwner,
private dir: TcbDirectiveMetadata,
private customFormControlType: CustomFormControlType | null,
private directiveIndex?: number,
) {
super();
}
Expand Down Expand Up @@ -78,7 +79,7 @@ export class TcbDirectiveCtorOp extends TcbOp {
this.dir.matchSource === MatchSource.HostDirective
? ExpressionIdentifier.HOST_DIRECTIVE
: ExpressionIdentifier.DIRECTIVE;
id.addExpressionIdentifier(identifier).addParseSpanInfo(span);
id.addExpressionIdentifier(identifier, this.directiveIndex).addParseSpanInfo(span);

for (const attr of boundAttrs) {
// Skip text attributes if configured to do so.
Expand Down
3 changes: 2 additions & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/src/ops/directive_type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export abstract class TcbDirectiveTypeOpBase extends TcbOp {
protected scope: Scope,
protected node: DirectiveOwner,
protected dir: TcbDirectiveMetadata,
protected directiveIndex?: number,
) {
super();
}
Expand Down Expand Up @@ -64,7 +65,7 @@ export abstract class TcbDirectiveTypeOpBase extends TcbOp {
? ExpressionIdentifier.HOST_DIRECTIVE
: ExpressionIdentifier.DIRECTIVE;
const id = new TcbExpr(this.tcb.allocateId())
.addExpressionIdentifier(identifier)
.addExpressionIdentifier(identifier, this.directiveIndex)

@crisbeto crisbeto Apr 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that the directive index is stable enough for us to rely on. Some alternatives we can consider:

  1. Class name and some other field (maybe index?) to disambiguate.
  2. ref.node.key field which is supposed to uniquely identify a directive.

.addParseSpanInfo(span);
this.scope.addStatement(declareVariable(id, type));
return id;
Expand Down
20 changes: 12 additions & 8 deletions packages/compiler-cli/src/ngtsc/typecheck/src/ops/scope.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,9 @@ export class Scope {
}

const dirMap = new Map<TcbDirectiveMetadata, number>();
for (const dir of directives) {
this.appendDirectiveInputs(dir, node, dirMap, directives);
for (let i = 0; i < directives.length; i++) {
const dir = directives[i];
this.appendDirectiveInputs(dir, node, dirMap, directives, i);
}
this.directiveOpMap.set(node, dirMap);

Expand Down Expand Up @@ -680,8 +681,9 @@ export class Scope {

if (directives !== null && directives.length > 0) {
const dirMap = new Map<TcbDirectiveMetadata, number>();
for (const dir of directives) {
this.appendDirectiveInputs(dir, node, dirMap, directives);
for (let i = 0; i < directives.length; i++) {
const dir = directives[i];
this.appendDirectiveInputs(dir, node, dirMap, directives, i);

for (const propertyName of dir.inputs.propertyNames) {
claimedInputs.add(propertyName);
Expand Down Expand Up @@ -750,11 +752,12 @@ export class Scope {
node: TmplAstElement | TmplAstTemplate | TmplAstComponent | TmplAstDirective,
dirMap: Map<TcbDirectiveMetadata, number>,
allDirectiveMatches: TcbDirectiveMetadata[],
directiveIndex?: number,
): void {
const nodeIsFormControl = isFormControl(allDirectiveMatches);
const customFormControlType = nodeIsFormControl ? getCustomFieldDirectiveType(dir) : null;

const directiveOp = this.getDirectiveOp(dir, node, customFormControlType);
const directiveOp = this.getDirectiveOp(dir, node, customFormControlType, directiveIndex);
const dirIndex = this.opQueue.push(directiveOp) - 1;
dirMap.set(dir, dirIndex);

Expand All @@ -779,21 +782,22 @@ export class Scope {
dir: TcbDirectiveMetadata,
node: DirectiveOwner,
customFieldType: CustomFormControlType | null,
directiveIndex?: number,
): TcbOp {
if (!dir.isGeneric) {
// The most common case is that when a directive is not generic, we use the normal
// `TcbNonDirectiveTypeOp`.
return new TcbNonGenericDirectiveTypeOp(this.tcb, this, node, dir);
return new TcbNonGenericDirectiveTypeOp(this.tcb, this, node, dir, directiveIndex);
} else if (!dir.requiresInlineTypeCtor || this.tcb.env.config.useInlineTypeConstructors) {
// For generic directives, we use a type constructor to infer types. If a directive requires
// an inline type constructor, then inlining must be available to use the
// `TcbDirectiveCtorOp`. If not we, we fallback to using `any` – see below.
return new TcbDirectiveCtorOp(this.tcb, this, node, dir, customFieldType);
return new TcbDirectiveCtorOp(this.tcb, this, node, dir, customFieldType, directiveIndex);
}

// If inlining is not available, then we give up on inferring the generic params, and use
// `any` type for the directive's generic parameters.
return new TcbGenericDirectiveTypeWithAnyParamsOp(this.tcb, this, node, dir);
return new TcbGenericDirectiveTypeWithAnyParamsOp(this.tcb, this, node, dir, directiveIndex);
}

private appendSelectorlessDirectives(
Expand Down
152 changes: 37 additions & 115 deletions packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import {
findAllMatchingNodes,
findFirstMatchingNode,
hasExpressionIdentifier,
readDirectiveIdFromComment,
} from './comments';
import {TypeCheckData} from './context';
import {isAccessExpression, isDirectiveDeclaration} from './ts_util';
Expand Down Expand Up @@ -207,36 +208,52 @@ export class SymbolBuilder {
templateNode: TmplAstElement | TmplAstTemplate | TmplAstComponent | TmplAstDirective,
): DirectiveSymbol[] {
const elementSourceSpan = templateNode.startSourceSpan ?? templateNode.sourceSpan;
const nodes = findAllMatchingNodes(this.typeCheckBlock, {
withSpan: elementSourceSpan,
filter: isDirectiveDeclaration,
});
const symbols: DirectiveSymbol[] = [];
const seenDirectives = new Set<ts.ClassDeclaration>();
const boundDirectives = this.typeCheckData.boundTarget.getDirectivesOfNode(templateNode) ?? [];

let boundDirectives = this.typeCheckData.boundTarget.getDirectivesOfNode(templateNode) ?? [];
let symbols = this.getDirectiveSymbolsForDirectives(boundDirectives, elementSourceSpan);

// 'getDirectivesOfNode' will not return the directives intended for an element
// on a microsyntax template, for example '<div *ngFor="let user of users;" dir>',
// the 'dir' will be skipped, but it's needed in language service.
if (!(templateNode instanceof TmplAstDirective)) {
const firstChild = templateNode.children?.[0];
if (firstChild instanceof TmplAstElement) {
const firstChild = templateNode.children.find(
(c): c is TmplAstElement => c instanceof TmplAstElement,
);
if (firstChild !== undefined) {
const isMicrosyntaxTemplate =
templateNode instanceof TmplAstTemplate &&
sourceSpanEqual(firstChild.sourceSpan, templateNode.sourceSpan);
if (isMicrosyntaxTemplate) {
const firstChildDirectives =
this.typeCheckData.boundTarget.getDirectivesOfNode(firstChild);
if (firstChildDirectives !== null && boundDirectives.length > 0) {
boundDirectives = boundDirectives.concat(firstChildDirectives);
} else if (firstChildDirectives !== null) {
boundDirectives = firstChildDirectives;
if (firstChildDirectives !== null) {
const childSymbols = this.getDirectiveSymbolsForDirectives(
firstChildDirectives,
elementSourceSpan,
);
// Merge symbols, avoiding duplicates
for (const symbol of childSymbols) {
if (!symbols.some((s) => s.ref.node === symbol.ref.node)) {
symbols.push(symbol);
}
}
}
}
}
}

return symbols;
}

private getDirectiveSymbolsForDirectives(
boundDirectives: TypeCheckableDirectiveMeta[],
span: ParseSourceSpan,
): DirectiveSymbol[] {
const nodes = findAllMatchingNodes(this.typeCheckBlock, {
withSpan: span,
filter: isDirectiveDeclaration,
});

const hostDirectiveMap = new Map<ts.Node, HostDirectiveMeta>();
for (const d of boundDirectives) {
if (d.hostDirectives) {
Expand All @@ -248,33 +265,14 @@ export class SymbolBuilder {
}
}

for (let i = 0; i < nodes.length; i++) {
const node = nodes[i];

let nodeName: string | null = null;
let typeNode = ts.isTypeNode(node)
? node
: ts.isIdentifier(node) && node.parent && ts.isVariableDeclaration(node.parent)
? node.parent.type
: null;
if (typeNode && ts.isTypeReferenceNode(typeNode)) {
const typeName = typeNode.typeName;
nodeName = ts.isIdentifier(typeName) ? typeName.text : typeName.right.text;
} else if (typeNode && ts.isIntersectionTypeNode(typeNode)) {
const first = typeNode.types[0];
if (ts.isTypeReferenceNode(first)) {
const typeName = first.typeName;
nodeName = ts.isIdentifier(typeName) ? typeName.text : typeName.right.text;
}
}

// Match by name with index fallback
let meta = boundDirectives[i];
if (nodeName) {
meta =
boundDirectives.find((m) => m.ref.node.name && m.ref.node.name.text === nodeName) ?? meta;
}
const symbols: DirectiveSymbol[] = [];
const seenDirectives = new Set<ts.ClassDeclaration>();
const sf = this.typeCheckBlock.getSourceFile();

for (const node of nodes) {
const id = readDirectiveIdFromComment(sf, node);
if (id === null) continue;
const meta = boundDirectives[id];
if (!meta) continue;

const declaration = meta.ref.node as unknown as ts.ClassDeclaration;
Expand Down Expand Up @@ -316,85 +314,9 @@ export class SymbolBuilder {
}
}

// Sort to ensure host directives appear first (matching test expectations)
symbols.sort((a, b) => {
if (a.matchSource === MatchSource.HostDirective && b.matchSource === MatchSource.Selector) {
return -1;
}
if (a.matchSource === MatchSource.Selector && b.matchSource === MatchSource.HostDirective) {
return 1;
}
return 0;
});

return symbols;
}

private getDirectiveMeta(
host: TmplAstTemplate | TmplAstElement | TmplAstComponent | TmplAstDirective,
directiveDeclaration: ts.ClassDeclaration,
): TypeCheckableDirectiveMeta | null {
let directives = this.typeCheckData.boundTarget.getDirectivesOfNode(host);

// `getDirectivesOfNode` will not return the directives intended for an element
// on a microsyntax template, for example `<div *ngFor="let user of users;" dir>`,
// the `dir` will be skipped, but it's needed in language service.
if (!(host instanceof TmplAstDirective)) {
const firstChild = host.children[0];
if (firstChild instanceof TmplAstElement) {
const isMicrosyntaxTemplate =
host instanceof TmplAstTemplate &&
sourceSpanEqual(firstChild.sourceSpan, host.sourceSpan);
if (isMicrosyntaxTemplate) {
const firstChildDirectives =
this.typeCheckData.boundTarget.getDirectivesOfNode(firstChild);
if (firstChildDirectives !== null && directives !== null) {
directives = directives.concat(firstChildDirectives);
} else {
directives = directives ?? firstChildDirectives;
}
}
}
}
if (directives === null) {
return null;
}

const directive = directives.find((m) =>
isSameDirectiveDeclaration(m.ref.node, directiveDeclaration),
);
if (directive) {
return directive;
}

const originalFile = (directiveDeclaration.getSourceFile() as MaybeSourceFileWithOriginalFile)[
NgOriginalFile
];

if (originalFile !== undefined) {
// This is a preliminary check ahead of a more expensive search
const hasPotentialCandidate = directives.find(
(m) => m.ref.node.name.text === directiveDeclaration.name?.text,
);

if (hasPotentialCandidate) {
// In case the TCB has been inlined,
// We will look for a matching class
// If we find one, we look for it in the directives array
const classWithSameName = findMatchingDirective(originalFile, directiveDeclaration);
if (classWithSameName !== null) {
return (
directives.find((m) => isSameDirectiveDeclaration(m.ref.node, classWithSameName)) ??
null
);
}
}
}

// Really nothing was found
return null;
}

private getDirectiveModule(declaration: ts.ClassDeclaration): ClassDeclaration | null {
const scope = this.componentScopeReader.getScopeForComponent(declaration as ClassDeclaration);
if (scope === null || scope.kind !== ComponentScopeKind.NgModule) {
Expand Down
Loading
Loading