Skip to content

Commit b0c1a90

Browse files
atscottAndrewKushnir
atscott
authored and
AndrewKushnir
committed
fix(compiler): Produce diagnositc if directive used in host binding is not exported (#49792)
The compiler currently does not check to make sure that directives in the host bindings are exported. These directives are part of the public API of the component so they do have to be. PR Close #49792
1 parent a40529a commit b0c1a90

File tree

8 files changed

+50
-19
lines changed

8 files changed

+50
-19
lines changed

‎packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,16 +112,16 @@ export class DecorationAnalyzer {
112112
/* i18nUseExternalIds */ true, this.bundle.enableI18nLegacyMessageIdFormat,
113113
/* usePoisonedData */ false,
114114
/* i18nNormalizeLineEndingsInICUs */ false, this.moduleResolver, this.cycleAnalyzer,
115-
CycleHandlingStrategy.UseRemoteScoping, this.refEmitter, NOOP_DEPENDENCY_TRACKER,
116-
this.injectableRegistry,
115+
CycleHandlingStrategy.UseRemoteScoping, this.refEmitter, this.referencesRegistry,
116+
NOOP_DEPENDENCY_TRACKER, this.injectableRegistry,
117117
/* semanticDepGraphUpdater */ null, !!this.compilerOptions.annotateForClosureCompiler,
118118
NOOP_PERF_RECORDER, this.hostDirectivesResolver),
119119

120120
// See the note in ngtsc about why this cast is needed.
121121
// clang-format off
122122
new DirectiveDecoratorHandler(
123123
this.reflectionHost, this.evaluator, this.fullRegistry, this.scopeRegistry,
124-
this.fullMetaReader, this.injectableRegistry, this.refEmitter, this.isCore, /* strictCtorDeps */ false,
124+
this.fullMetaReader, this.injectableRegistry, this.refEmitter, this.referencesRegistry, this.isCore, /* strictCtorDeps */ false,
125125
/* semanticDepGraphUpdater */ null,
126126
!!this.compilerOptions.annotateForClosureCompiler,
127127
// In ngcc we want to compile undecorated classes with Angular features. As of

‎packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {TypeCheckableDirectiveMeta, TypeCheckContext} from '../../../typecheck/a
2626
import {ExtendedTemplateChecker} from '../../../typecheck/extended/api';
2727
import {getSourceFile} from '../../../util/src/typescript';
2828
import {Xi18nContext} from '../../../xi18n';
29-
import {combineResolvers, compileDeclareFactory, compileNgFactoryDefField, compileResults, extractClassMetadata, extractSchemas, findAngularDecorator, forwardRefResolver, getDirectiveDiagnostics, getProviderDiagnostics, InjectableClassRegistry, isExpressionForwardReference, readBaseClass, resolveEnumValue, resolveImportedFile, resolveLiteral, resolveProvidersRequiringFactory, ResourceLoader, toFactoryMetadata, validateHostDirectives, wrapFunctionExpressionsInParens,} from '../../common';
29+
import {combineResolvers, compileDeclareFactory, compileNgFactoryDefField, compileResults, extractClassMetadata, extractSchemas, findAngularDecorator, forwardRefResolver, getDirectiveDiagnostics, getProviderDiagnostics, InjectableClassRegistry, isExpressionForwardReference, readBaseClass, ReferencesRegistry, resolveEnumValue, resolveImportedFile, resolveLiteral, resolveProvidersRequiringFactory, ResourceLoader, toFactoryMetadata, validateHostDirectives, wrapFunctionExpressionsInParens,} from '../../common';
3030
import {extractDirectiveMetadata, parseFieldArrayValue} from '../../directive';
3131
import {createModuleWithProvidersResolver, NgModuleSymbol} from '../../ng_module';
3232

@@ -57,7 +57,7 @@ export class ComponentDecoratorHandler implements
5757
private usePoisonedData: boolean, private i18nNormalizeLineEndingsInICUs: boolean,
5858
private moduleResolver: ModuleResolver, private cycleAnalyzer: CycleAnalyzer,
5959
private cycleHandlingStrategy: CycleHandlingStrategy, private refEmitter: ReferenceEmitter,
60-
private depTracker: DependencyTracker|null,
60+
private referencesRegistry: ReferencesRegistry, private depTracker: DependencyTracker|null,
6161
private injectableRegistry: InjectableClassRegistry,
6262
private semanticDepGraphUpdater: SemanticDepGraphUpdater|null,
6363
private annotateForClosureCompiler: boolean, private perf: PerfRecorder,
@@ -190,8 +190,8 @@ export class ComponentDecoratorHandler implements
190190
// @Component inherits @Directive, so begin by extracting the @Directive metadata and building
191191
// on it.
192192
const directiveResult = extractDirectiveMetadata(
193-
node, decorator, this.reflector, this.evaluator, this.refEmitter, this.isCore, flags,
194-
this.annotateForClosureCompiler,
193+
node, decorator, this.reflector, this.evaluator, this.refEmitter, this.referencesRegistry,
194+
this.isCore, flags, this.annotateForClosureCompiler,
195195
this.elementSchemaRegistry.getDefaultComponentElementName());
196196
if (directiveResult === undefined) {
197197
// `extractDirectiveMetadata` returns undefined when the @Directive has `jit: true`. In this

‎packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {NOOP_PERF_RECORDER} from '../../../perf';
2020
import {isNamedClassDeclaration, TypeScriptReflectionHost} from '../../../reflection';
2121
import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver, TypeCheckScopeRegistry} from '../../../scope';
2222
import {getDeclaration, makeProgram} from '../../../testing';
23-
import {InjectableClassRegistry, ResourceLoader, ResourceLoaderContext} from '../../common';
23+
import {InjectableClassRegistry, NoopReferencesRegistry, ResourceLoader, ResourceLoaderContext} from '../../common';
2424
import {ComponentDecoratorHandler} from '../src/handler';
2525

2626
export class StubResourceLoader implements ResourceLoader {
@@ -55,6 +55,7 @@ function setup(program: ts.Program, options: ts.CompilerOptions, host: ts.Compil
5555
const scopeRegistry = new LocalModuleScopeRegistry(
5656
metaRegistry, metaReader, dtsResolver, new ReferenceEmitter([]), null);
5757
const refEmitter = new ReferenceEmitter([]);
58+
const referencesRegistry = new NoopReferencesRegistry();
5859
const injectableRegistry = new InjectableClassRegistry(reflectionHost, /* isCore */ false);
5960
const resourceRegistry = new ResourceRegistry();
6061
const hostDirectivesResolver = new HostDirectivesResolver(metaReader);
@@ -85,6 +86,7 @@ function setup(program: ts.Program, options: ts.CompilerOptions, host: ts.Compil
8586
cycleAnalyzer,
8687
CycleHandlingStrategy.UseRemoteScoping,
8788
refEmitter,
89+
referencesRegistry,
8890
/* depTracker */ null,
8991
injectableRegistry,
9092
/* semanticDepGraphUpdater */ null,

‎packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {PerfEvent, PerfRecorder} from '../../../perf';
1717
import {ClassDeclaration, ClassMember, ClassMemberKind, Decorator, ReflectionHost} from '../../../reflection';
1818
import {LocalModuleScopeRegistry} from '../../../scope';
1919
import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerFlags, HandlerPrecedence, ResolveResult} from '../../../transform';
20-
import {compileDeclareFactory, compileNgFactoryDefField, compileResults, extractClassMetadata, findAngularDecorator, getDirectiveDiagnostics, getProviderDiagnostics, getUndecoratedClassWithAngularFeaturesDiagnostic, InjectableClassRegistry, isAngularDecorator, readBaseClass, resolveProvidersRequiringFactory, toFactoryMetadata, validateHostDirectives} from '../../common';
20+
import {compileDeclareFactory, compileNgFactoryDefField, compileResults, extractClassMetadata, findAngularDecorator, getDirectiveDiagnostics, getProviderDiagnostics, getUndecoratedClassWithAngularFeaturesDiagnostic, InjectableClassRegistry, isAngularDecorator, readBaseClass, ReferencesRegistry, resolveProvidersRequiringFactory, toFactoryMetadata, validateHostDirectives} from '../../common';
2121

2222
import {extractDirectiveMetadata} from './shared';
2323
import {DirectiveSymbol} from './symbol';
@@ -52,8 +52,8 @@ export class DirectiveDecoratorHandler implements
5252
private reflector: ReflectionHost, private evaluator: PartialEvaluator,
5353
private metaRegistry: MetadataRegistry, private scopeRegistry: LocalModuleScopeRegistry,
5454
private metaReader: MetadataReader, private injectableRegistry: InjectableClassRegistry,
55-
private refEmitter: ReferenceEmitter, private isCore: boolean,
56-
private strictCtorDeps: boolean,
55+
private refEmitter: ReferenceEmitter, private referencesRegistry: ReferencesRegistry,
56+
private isCore: boolean, private strictCtorDeps: boolean,
5757
private semanticDepGraphUpdater: SemanticDepGraphUpdater|null,
5858
private annotateForClosureCompiler: boolean,
5959
private compileUndecoratedClassesWithAngularFeatures: boolean, private perf: PerfRecorder) {}
@@ -95,8 +95,8 @@ export class DirectiveDecoratorHandler implements
9595
this.perf.eventCount(PerfEvent.AnalyzeDirective);
9696

9797
const directiveResult = extractDirectiveMetadata(
98-
node, decorator, this.reflector, this.evaluator, this.refEmitter, this.isCore, flags,
99-
this.annotateForClosureCompiler);
98+
node, decorator, this.reflector, this.evaluator, this.refEmitter, this.referencesRegistry,
99+
this.isCore, flags, this.annotateForClosureCompiler);
100100
if (directiveResult === undefined) {
101101
return {};
102102
}

‎packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {ClassPropertyMapping, HostDirectiveMeta} from '../../../metadata';
1515
import {DynamicValue, EnumValue, PartialEvaluator, ResolvedValue} from '../../../partial_evaluator';
1616
import {ClassDeclaration, ClassMember, ClassMemberKind, Decorator, filterToMembersWithDecorator, isNamedClassDeclaration, ReflectionHost, reflectObjectLiteral} from '../../../reflection';
1717
import {HandlerFlags} from '../../../transform';
18-
import {createSourceSpan, createValueHasWrongTypeError, forwardRefResolver, getConstructorDependencies, toR3Reference, tryUnwrapForwardRef, unwrapConstructorDependencies, unwrapExpression, validateConstructorDependencies, wrapFunctionExpressionsInParens, wrapTypeReference} from '../../common';
18+
import {createSourceSpan, createValueHasWrongTypeError, forwardRefResolver, getConstructorDependencies, ReferencesRegistry, toR3Reference, tryUnwrapForwardRef, unwrapConstructorDependencies, unwrapExpression, validateConstructorDependencies, wrapFunctionExpressionsInParens, wrapTypeReference} from '../../common';
1919

2020
const EMPTY_OBJECT: {[key: string]: string} = {};
2121
const QUERY_TYPES = new Set([
@@ -33,7 +33,8 @@ const QUERY_TYPES = new Set([
3333
*/
3434
export function extractDirectiveMetadata(
3535
clazz: ClassDeclaration, decorator: Readonly<Decorator|null>, reflector: ReflectionHost,
36-
evaluator: PartialEvaluator, refEmitter: ReferenceEmitter, isCore: boolean, flags: HandlerFlags,
36+
evaluator: PartialEvaluator, refEmitter: ReferenceEmitter,
37+
referencesRegistry: ReferencesRegistry, isCore: boolean, flags: HandlerFlags,
3738
annotateForClosureCompiler: boolean, defaultSelector: string|null = null): {
3839
decorator: Map<string, ts.Expression>,
3940
metadata: R3DirectiveMetadata,
@@ -191,6 +192,13 @@ export function extractDirectiveMetadata(
191192
const hostDirectives =
192193
rawHostDirectives === null ? null : extractHostDirectives(rawHostDirectives, evaluator);
193194

195+
if (hostDirectives !== null) {
196+
// The template type-checker will need to import host directive types, so add them
197+
// as referenced by `clazz`. This will ensure that libraries are required to export
198+
// host directives which are visible from publicly exported components.
199+
referencesRegistry.add(clazz, ...hostDirectives.map(hostDir => hostDir.directive));
200+
}
201+
194202
const metadata: R3DirectiveMetadata = {
195203
name: clazz.name.text,
196204
deps: ctorDeps,

‎packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {NOOP_PERF_RECORDER} from '../../../perf';
1717
import {ClassDeclaration, isNamedClassDeclaration, TypeScriptReflectionHost} from '../../../reflection';
1818
import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../../scope';
1919
import {getDeclaration, makeProgram} from '../../../testing';
20-
import {InjectableClassRegistry} from '../../common';
20+
import {InjectableClassRegistry, NoopReferencesRegistry} from '../../common';
2121
import {DirectiveDecoratorHandler} from '../index';
2222

2323
runInEachFileSystem(() => {
@@ -166,13 +166,14 @@ runInEachFileSystem(() => {
166166
const metaReader = new LocalMetadataRegistry();
167167
const dtsReader = new DtsMetadataReader(checker, reflectionHost);
168168
const refEmitter = new ReferenceEmitter([]);
169+
const referenceRegistry = new NoopReferencesRegistry();
169170
const scopeRegistry = new LocalModuleScopeRegistry(
170171
metaReader, new CompoundMetadataReader([metaReader, dtsReader]),
171172
new MetadataDtsModuleScopeResolver(dtsReader, null), refEmitter, null);
172173
const injectableRegistry = new InjectableClassRegistry(reflectionHost, /* isCore */ false);
173174
const handler = new DirectiveDecoratorHandler(
174175
reflectionHost, evaluator, scopeRegistry, scopeRegistry, metaReader, injectableRegistry,
175-
refEmitter,
176+
refEmitter, referenceRegistry,
176177
/*isCore*/ false,
177178
/*strictCtorDeps*/ false,
178179
/*semanticDepGraphUpdater*/ null,

‎packages/compiler-cli/src/ngtsc/core/src/compiler.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1043,7 +1043,7 @@ export class NgCompiler {
10431043
this.options.i18nUseExternalIds !== false,
10441044
this.options.enableI18nLegacyMessageIdFormat !== false, this.usePoisonedData,
10451045
this.options.i18nNormalizeLineEndingsInICUs === true, this.moduleResolver,
1046-
this.cycleAnalyzer, cycleHandlingStrategy, refEmitter,
1046+
this.cycleAnalyzer, cycleHandlingStrategy, refEmitter, referencesRegistry,
10471047
this.incrementalCompilation.depGraph, injectableRegistry, semanticDepGraphUpdater,
10481048
this.closureCompilerEnabled, this.delegatingPerfRecorder, hostDirectivesResolver),
10491049

@@ -1052,7 +1052,7 @@ export class NgCompiler {
10521052
// clang-format off
10531053
new DirectiveDecoratorHandler(
10541054
reflector, evaluator, metaRegistry, ngModuleScopeRegistry, metaReader,
1055-
injectableRegistry, refEmitter, isCore, strictCtorDeps, semanticDepGraphUpdater,
1055+
injectableRegistry, refEmitter,referencesRegistry, isCore, strictCtorDeps, semanticDepGraphUpdater,
10561056
this.closureCompilerEnabled, /** compileUndecoratedClassesWithAngularFeatures */ false,
10571057
this.delegatingPerfRecorder,
10581058
) as Readonly<DecoratorHandler<unknown, unknown, SemanticSymbol | null,unknown>>,

‎packages/compiler-cli/test/ngtsc/ngtsc_spec.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6088,6 +6088,26 @@ function allTests(os: string) {
60886088
expect(ts.isClassDeclaration(id.parent)).toBe(true);
60896089
});
60906090

6091+
it('should report an error when a visible host directive is not exported', () => {
6092+
env.tsconfig({'flatModuleOutFile': 'flat.js'});
6093+
env.write('test.ts', `
6094+
import {Directive, NgModule} from '@angular/core';
6095+
6096+
@Directive({
6097+
standalone: true,
6098+
})
6099+
class HostDir {}
6100+
6101+
// The directive is not exported.
6102+
@Directive({selector: 'test', hostDirectives: [HostDir]})
6103+
export class Dir {}
6104+
`);
6105+
6106+
const errors = env.driveDiagnostics();
6107+
expect(errors.length).toBe(1);
6108+
expect(errors[0].code).toBe(ngErrorCode(ErrorCode.SYMBOL_NOT_EXPORTED));
6109+
});
6110+
60916111
it('should report an error when a deeply visible directive is not exported', () => {
60926112
env.tsconfig({'flatModuleOutFile': 'flat.js'});
60936113
env.write('test.ts', `

0 commit comments

Comments
 (0)