Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@ import {
makeBindingParser,
MatchSource,
outputAst as o,
MaybeForwardRefExpression,
R3ComponentDeferMetadata,
R3ComponentMetadata,
R3QueryMetadata,
R3DeferPerComponentDependency,
R3DirectiveDependencyMetadata,
R3NgModuleDependencyMetadata,
Expand Down Expand Up @@ -1392,6 +1394,7 @@ export class ComponentDecoratorHandler implements DecoratorHandler<
analysis,
eagerlyUsed,
);
this.checkDeferredQueryDiagnostics(analysis, data, eagerlyUsed, diagnostics);
data.hasDirectiveDependencies =
!analysis.meta.isStandalone ||
allDependencies.some(({kind, ref}) => {
Expand Down Expand Up @@ -2369,6 +2372,76 @@ export class ComponentDecoratorHandler implements DecoratorHandler<
});
}

/**
* Emits a diagnostic for each view/content query whose predicate is a component class that is
* only used inside `@defer` blocks. Such a reference prevents the component from being lazily
* loaded and the developer should switch to a string-based query with a local ref instead.
*/
private checkDeferredQueryDiagnostics(
analysis: Readonly<ComponentAnalysisData>,
data: ComponentResolutionData,
eagerlyUsedDecls: Set<ClassDeclaration>,
diagnostics: ts.Diagnostic[],
): void {
// Build a map of class decl → dep for components that appear exclusively inside @defer blocks.
const deferOnlyDeps = new Map<ClassDeclaration, DeferredComponentDependency>();
for (const [, deps] of data.deferPerBlockDependencies) {
for (const dep of deps) {
if (!eagerlyUsedDecls.has(dep.declaration.node)) {
deferOnlyDeps.set(dep.declaration.node, dep);
}
}
}
if (deferOnlyDeps.size === 0) return;

const checkQueryList = (queries: R3QueryMetadata[]) => {
for (const query of queries) {
// string[] predicate means viewChild('ref') — already the good path, skip it.
if (Array.isArray(query.predicate)) continue;

// We gotta unwrap the compiler IR to get back to the real TS node.
const expr = (query.predicate as MaybeForwardRefExpression<o.Expression>).expression;
if (!(expr instanceof o.WrappedNodeExpr)) continue;

// This could be a plain identifier (Cmp) or a qualified name (ns.Cmp): let's handle both.
const tsNode = (expr as o.WrappedNodeExpr<ts.Expression>).node;
const identifier =
ts.isIdentifier(tsNode)
? tsNode
: ts.isPropertyAccessExpression(tsNode) && ts.isIdentifier(tsNode.name)
? tsNode.name
: null;
if (identifier === null) continue;

// Resolve the identifier all the way back to its class declaration.
const decl = this.reflector.getDeclarationOfIdentifier(identifier);
if (decl === null || !isNamedClassDeclaration(decl.node)) continue;

const dep = deferOnlyDeps.get(decl.node as ClassDeclaration);
if (dep === undefined) continue;

// We got a hit — this class lives exclusively behind @defer but is used as a class predicate,
// which keeps the import eager and kills the whole point of deferring it.
const className = (decl.node as ts.ClassDeclaration).name!.text;
const selector = this.metaReader.getDirectiveMetadata(dep.declaration)?.selector;
const selectorHint = selector ? ` \`<${selector} #ref />\`` : ' the element';
diagnostics.push(
makeDiagnostic(
ErrorCode.DEFERRED_COMPONENT_USED_IN_QUERY,
tsNode,
`Query \`${query.propertyName}\` references \`${className}\`, which is only used ` +
`inside a \`@defer\` block. This type reference prevents \`${className}\` from ` +
`being lazily loaded. Add a template reference variable to${selectorHint} and use ` +
`a string-based query instead: \`viewChild<${className}>('ref')\`.`,
),
);
}
};

checkQueryList(analysis.meta.viewQueries);
checkQueryList(analysis.meta.queries);
}

/**
* Resolves information about defer blocks dependencies to make it
* available for the final `compile` step.
Expand Down
7 changes: 7 additions & 0 deletions packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,13 @@ export enum ErrorCode {
*/
CONFLICTING_CONTENT_AND_PROPERTY = 8029,

/**
* A `viewChild` or `contentChild` query uses a component class as its predicate, but that
* component is only used inside a `@defer` block. The class reference prevents the component
* from being lazily loaded. Use a string-based query with a template reference variable instead.
*/
DEFERRED_COMPONENT_USED_IN_QUERY = 8030,

/**
* A two way binding in a template has an incorrect syntax,
* parentheses outside brackets. For example:
Expand Down
197 changes: 194 additions & 3 deletions packages/compiler-cli/test/ngtsc/defer_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -801,11 +801,11 @@ runInEachFileSystem(() => {
\`,
})
export class TestCmp {
// Type-only reference
// Type-only reference (fine — no class predicate)
query = viewChild<Cmp>('ref');

// Directy reference
otherQuery = viewChild(Cmp);
// Direct value reference prevents lazy loading without triggering a query diagnostic
static readonly cmpType = Cmp;
}
`,
);
Expand Down Expand Up @@ -1722,6 +1722,7 @@ runInEachFileSystem(() => {
});

it('should count whitespace as a root node when preserveWhitespaces is enabled', () => {

env.write(
'/test.ts',
`
Expand All @@ -1742,5 +1743,195 @@ runInEachFileSystem(() => {
);
});
});

describe('DEFERRED_COMPONENT_USED_IN_QUERY diagnostic', () => {
it('should report when viewChild uses a defer-only component class as predicate', () => {
env.write(
'deferred-cmp.ts',
`
import {Component} from '@angular/core';
@Component({selector: 'deferred-cmp', template: ''})
export class DeferredCmp {}
`,
);

env.write(
'test.ts',
`
import {Component, viewChild} from '@angular/core';
import {DeferredCmp} from './deferred-cmp';

@Component({
imports: [DeferredCmp],
template: \`
@defer {
<deferred-cmp />
}
\`,
})
export class AppCmp {
child = viewChild(DeferredCmp);
}
`,
);

const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DEFERRED_COMPONENT_USED_IN_QUERY));
expect(diags[0].messageText as string).toContain('DeferredCmp');
expect(diags[0].messageText as string).toContain('@defer');
});

it('should not report when viewChild uses a string-based predicate', () => {
env.write(
'deferred-cmp.ts',
`
import {Component} from '@angular/core';
@Component({selector: 'deferred-cmp', template: ''})
export class DeferredCmp {}
`,
);

env.write(
'test.ts',
`
import {Component, viewChild} from '@angular/core';
import {DeferredCmp} from './deferred-cmp';

@Component({
imports: [DeferredCmp],
template: \`
@defer {
<deferred-cmp #ref />
}
\`,
})
export class AppCmp {
child = viewChild<DeferredCmp>('ref');
}
`,
);

const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});

it('should not report when the component is also used outside the defer block', () => {
env.write(
'some-cmp.ts',
`
import {Component} from '@angular/core';
@Component({selector: 'some-cmp', template: ''})
export class SomeCmp {}
`,
);

env.write(
'test.ts',
`
import {Component, viewChild} from '@angular/core';
import {SomeCmp} from './some-cmp';

@Component({
imports: [SomeCmp],
template: \`
<some-cmp />
@defer {
<some-cmp />
}
\`,
})
export class AppCmp {
child = viewChild(SomeCmp);
}
`,
);

const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});

it('should report when contentChild uses a defer-only component class as predicate', () => {
env.write(
'deferred-cmp.ts',
`
import {Component} from '@angular/core';
@Component({selector: 'deferred-cmp', template: ''})
export class DeferredCmp {}
`,
);

env.write(
'test.ts',
`
import {Component, contentChild} from '@angular/core';
import {DeferredCmp} from './deferred-cmp';

@Component({
imports: [DeferredCmp],
template: \`
@defer {
<deferred-cmp />
}
\`,
})
export class AppCmp {
child = contentChild(DeferredCmp);
}
`,
);

const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DEFERRED_COMPONENT_USED_IN_QUERY));
});

it('should report one diagnostic per offending query', () => {
env.write(
'deferred-a.ts',
`
import {Component} from '@angular/core';
@Component({selector: 'deferred-a', template: ''})
export class DeferredA {}
`,
);

env.write(
'deferred-b.ts',
`
import {Component} from '@angular/core';
@Component({selector: 'deferred-b', template: ''})
export class DeferredB {}
`,
);

env.write(
'test.ts',
`
import {Component, viewChild} from '@angular/core';
import {DeferredA} from './deferred-a';
import {DeferredB} from './deferred-b';

@Component({
imports: [DeferredA, DeferredB],
template: \`
@defer {
<deferred-a />
<deferred-b />
}
\`,
})
export class AppCmp {
childA = viewChild(DeferredA);
childB = viewChild(DeferredB);
}
`,
);

const diags = env.driveDiagnostics();
expect(diags.length).toBe(2);
expect(diags.every((d) => d.code === ngErrorCode(ErrorCode.DEFERRED_COMPONENT_USED_IN_QUERY))).toBeTrue();
});
});
});
});
Loading