Skip to content

Commit c439d69

Browse files
crisbetoalxhub
authored andcommitted
fix(compiler-cli): symbol builder duplicating host directives (#61240)
The template symbol builder works by finding the variables referring to template AST nodes with specific offsets and resolving them to directives. Afterwards it goes through the directives and resolves their host directives. The problem is that host directives are added with the exact same offsets as their host which means they get added once initially and again when resolving host directives. These changes resolve the issue by de-duplicating them. PR Close #61240
1 parent 8f9a21a commit c439d69

File tree

2 files changed

+82
-12
lines changed

2 files changed

+82
-12
lines changed

packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ export class SymbolBuilder {
172172
filter: isDirectiveDeclaration,
173173
});
174174
const symbols: DirectiveSymbol[] = [];
175+
const seenDirectives = new Set<ts.ClassDeclaration>();
175176

176177
for (const node of nodes) {
177178
const symbol = this.getSymbolOfTsNode(node.parent);
@@ -183,13 +184,16 @@ export class SymbolBuilder {
183184
continue;
184185
}
185186

186-
const meta = this.getDirectiveMeta(element, symbol.tsSymbol.valueDeclaration);
187+
const declaration = symbol.tsSymbol.valueDeclaration;
188+
const meta = this.getDirectiveMeta(element, declaration);
187189

188-
if (meta !== null && meta.selector !== null) {
189-
const ref = new Reference<ClassDeclaration>(symbol.tsSymbol.valueDeclaration as any);
190+
// Host directives will be added as identifiers with the same offset as the host
191+
// which means that they'll get added twice. De-duplicate them to avoid confusion.
192+
if (meta !== null && !seenDirectives.has(declaration)) {
193+
const ref = new Reference<ClassDeclaration>(declaration as ClassDeclaration);
190194

191195
if (meta.hostDirectives !== null) {
192-
this.addHostDirectiveSymbols(element, meta.hostDirectives, symbols);
196+
this.addHostDirectiveSymbols(element, meta.hostDirectives, symbols, seenDirectives);
193197
}
194198

195199
const directiveSymbol: DirectiveSymbol = {
@@ -198,14 +202,15 @@ export class SymbolBuilder {
198202
tsSymbol: symbol.tsSymbol,
199203
selector: meta.selector,
200204
isComponent: meta.isComponent,
201-
ngModule: this.getDirectiveModule(symbol.tsSymbol.valueDeclaration),
205+
ngModule: this.getDirectiveModule(declaration),
202206
kind: SymbolKind.Directive,
203207
isStructural: meta.isStructural,
204208
isInScope: true,
205209
isHostDirective: false,
206210
};
207211

208212
symbols.push(directiveSymbol);
213+
seenDirectives.add(declaration);
209214
}
210215
}
211216

@@ -216,22 +221,25 @@ export class SymbolBuilder {
216221
host: TmplAstTemplate | TmplAstElement,
217222
hostDirectives: HostDirectiveMeta[],
218223
symbols: DirectiveSymbol[],
224+
seenDirectives: Set<ts.ClassDeclaration>,
219225
): void {
220226
for (const current of hostDirectives) {
221227
if (!isHostDirectiveMetaForGlobalMode(current)) {
222228
throw new Error('Impossible state: typecheck code path in local compilation mode.');
223229
}
224230

225-
if (!ts.isClassDeclaration(current.directive.node)) {
231+
const node = current.directive.node;
232+
233+
if (!ts.isClassDeclaration(node) || seenDirectives.has(node)) {
226234
continue;
227235
}
228236

229-
const symbol = this.getSymbolOfTsNode(current.directive.node);
230-
const meta = this.getDirectiveMeta(host, current.directive.node);
237+
const symbol = this.getSymbolOfTsNode(node);
238+
const meta = this.getDirectiveMeta(host, node);
231239

232240
if (meta !== null && symbol !== null && isSymbolWithValueDeclaration(symbol.tsSymbol)) {
233241
if (meta.hostDirectives !== null) {
234-
this.addHostDirectiveSymbols(host, meta.hostDirectives, symbols);
242+
this.addHostDirectiveSymbols(host, meta.hostDirectives, symbols, seenDirectives);
235243
}
236244

237245
const directiveSymbol: DirectiveSymbol = {
@@ -243,13 +251,14 @@ export class SymbolBuilder {
243251
exposedOutputs: current.outputs,
244252
selector: meta.selector,
245253
isComponent: meta.isComponent,
246-
ngModule: this.getDirectiveModule(current.directive.node),
254+
ngModule: this.getDirectiveModule(node),
247255
kind: SymbolKind.Directive,
248256
isStructural: meta.isStructural,
249257
isInScope: true,
250258
};
251259

252260
symbols.push(directiveSymbol);
261+
seenDirectives.add(node);
253262
}
254263
}
255264
}

packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import {
5656
setup as baseTestSetup,
5757
TypeCheckingTarget,
5858
createNgCompilerForFile,
59+
TestDirective,
5960
} from '../testing';
6061
import {TsCreateProgramDriver} from '../../program_driver';
6162
import {findNodeInFile} from '../src/tcb_util';
@@ -2286,7 +2287,7 @@ runInEachFileSystem(() => {
22862287
template: '<div *foo></div>',
22872288
})
22882289
class MyCmp {}
2289-
}
2290+
}
22902291
}
22912292
22922293
if(true) {
@@ -2331,7 +2332,7 @@ runInEachFileSystem(() => {
23312332
template: '<div *foo></div>',
23322333
})
23332334
class MyCmp {}
2334-
}
2335+
}
23352336
}
23362337
`);
23372338

@@ -2350,6 +2351,66 @@ runInEachFileSystem(() => {
23502351
expect(symbol.directives.length).toBe(1);
23512352
expect(symbol.directives[0].selector).toBe('[foo]');
23522353
});
2354+
2355+
it('should return the correct amount of directives when a host directive with a selector is applied', () => {
2356+
const fileName = absoluteFrom('/main.ts');
2357+
const depFile = absoluteFrom('/dep.ts');
2358+
const depHostFile = absoluteFrom('/dep-host.ts');
2359+
const depInnerHostFile = absoluteFrom('/dep-inner-host.ts');
2360+
const dep: TestDirective = {
2361+
name: 'Dep',
2362+
file: depFile,
2363+
selector: 'dep',
2364+
type: 'directive',
2365+
isStandalone: true,
2366+
isComponent: true,
2367+
hostDirectives: [
2368+
{
2369+
directive: {
2370+
name: 'DepHost',
2371+
file: depHostFile,
2372+
selector: 'dep-host', // <-- Note: this is necessary to hit the specific code path
2373+
type: 'directive',
2374+
isStandalone: true,
2375+
hostDirectives: [
2376+
{
2377+
directive: {
2378+
name: 'DepInnerHost',
2379+
file: depInnerHostFile,
2380+
selector: 'dep-inner-host', // <-- Note: this is necessary to hit the specific code path
2381+
type: 'directive',
2382+
isStandalone: true,
2383+
},
2384+
},
2385+
],
2386+
},
2387+
},
2388+
],
2389+
};
2390+
2391+
const {program, templateTypeChecker} = setup([
2392+
{fileName, templates: {'Cmp': '<dep/>'}, declarations: [dep]},
2393+
{fileName: depFile, source: 'export class Dep {}', templates: {}},
2394+
{fileName: depHostFile, source: 'export class DepHost {}', templates: {}},
2395+
{fileName: depInnerHostFile, source: 'export class DepInnerHost {}', templates: {}},
2396+
]);
2397+
const sf = getSourceFileOrError(program, fileName);
2398+
const cmp = getClass(sf, 'Cmp');
2399+
const nodes = templateTypeChecker.getTemplate(cmp)!;
2400+
const element = nodes[0] as TmplAstElement;
2401+
const symbol = templateTypeChecker.getSymbolOfNode(element, cmp)!;
2402+
assertElementSymbol(symbol);
2403+
expect(
2404+
symbol.directives.map((d) => ({
2405+
name: d.ref.node.name.text,
2406+
isHostDirective: d.isHostDirective,
2407+
})),
2408+
).toEqual([
2409+
{name: 'DepInnerHost', isHostDirective: true},
2410+
{name: 'DepHost', isHostDirective: true},
2411+
{name: 'Dep', isHostDirective: false},
2412+
]);
2413+
});
23532414
});
23542415
});
23552416

0 commit comments

Comments
 (0)