Skip to content

Commit e949548

Browse files
atscottAndrewKushnir
atscott
authored and
AndrewKushnir
committed
fix(compiler): Produce diagnositc if directive used in host binding is not exported (#49527)
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 #49527
1 parent 8a75a8a commit e949548

File tree

7 files changed

+47
-16
lines changed

7 files changed

+47
-16
lines changed

‎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, parseFieldStringArrayValue} from '../../directive';
3131
import {createModuleWithProvidersResolver, NgModuleSymbol} from '../../ng_module';
3232

@@ -56,7 +56,7 @@ export class ComponentDecoratorHandler implements
5656
private usePoisonedData: boolean, private i18nNormalizeLineEndingsInICUs: boolean,
5757
private moduleResolver: ModuleResolver, private cycleAnalyzer: CycleAnalyzer,
5858
private cycleHandlingStrategy: CycleHandlingStrategy, private refEmitter: ReferenceEmitter,
59-
private depTracker: DependencyTracker|null,
59+
private referencesRegistry: ReferencesRegistry, private depTracker: DependencyTracker|null,
6060
private injectableRegistry: InjectableClassRegistry,
6161
private semanticDepGraphUpdater: SemanticDepGraphUpdater|null,
6262
private annotateForClosureCompiler: boolean, private perf: PerfRecorder,
@@ -194,8 +194,8 @@ export class ComponentDecoratorHandler implements
194194
// @Component inherits @Directive, so begin by extracting the @Directive metadata and building
195195
// on it.
196196
const directiveResult = extractDirectiveMetadata(
197-
node, decorator, this.reflector, this.evaluator, this.refEmitter, this.isCore, flags,
198-
this.annotateForClosureCompiler,
197+
node, decorator, this.reflector, this.evaluator, this.refEmitter, this.referencesRegistry,
198+
this.isCore, flags, this.annotateForClosureCompiler,
199199
this.elementSchemaRegistry.getDefaultComponentElementName());
200200
if (directiveResult === undefined) {
201201
// `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, private perf: PerfRecorder) {}
5959

@@ -93,8 +93,8 @@ export class DirectiveDecoratorHandler implements
9393
this.perf.eventCount(PerfEvent.AnalyzeDirective);
9494

9595
const directiveResult = extractDirectiveMetadata(
96-
node, decorator, this.reflector, this.evaluator, this.refEmitter, this.isCore, flags,
97-
this.annotateForClosureCompiler);
96+
node, decorator, this.reflector, this.evaluator, this.refEmitter, this.referencesRegistry,
97+
this.isCore, flags, this.annotateForClosureCompiler);
9898
if (directiveResult === undefined) {
9999
return {};
100100
}

‎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, InputMapping} from '../../../me
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,
@@ -187,6 +188,13 @@ export function extractDirectiveMetadata(
187188
const hostDirectives =
188189
rawHostDirectives === null ? null : extractHostDirectives(rawHostDirectives, evaluator);
189190

191+
if (hostDirectives !== null) {
192+
// The template type-checker will need to import host directive types, so add them
193+
// as referenced by `clazz`. This will ensure that libraries are required to export
194+
// host directives which are visible from publicly exported components.
195+
referencesRegistry.add(clazz, ...hostDirectives.map(hostDir => hostDir.directive));
196+
}
197+
190198
const metadata: R3DirectiveMetadata = {
191199
name: clazz.name.text,
192200
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
@@ -1037,7 +1037,7 @@ export class NgCompiler {
10371037
this.options.i18nUseExternalIds !== false,
10381038
this.options.enableI18nLegacyMessageIdFormat !== false, this.usePoisonedData,
10391039
this.options.i18nNormalizeLineEndingsInICUs === true, this.moduleResolver,
1040-
this.cycleAnalyzer, cycleHandlingStrategy, refEmitter,
1040+
this.cycleAnalyzer, cycleHandlingStrategy, refEmitter, referencesRegistry,
10411041
this.incrementalCompilation.depGraph, injectableRegistry, semanticDepGraphUpdater,
10421042
this.closureCompilerEnabled, this.delegatingPerfRecorder, hostDirectivesResolver),
10431043

@@ -1046,7 +1046,7 @@ export class NgCompiler {
10461046
// clang-format off
10471047
new DirectiveDecoratorHandler(
10481048
reflector, evaluator, metaRegistry, ngModuleScopeRegistry, metaReader,
1049-
injectableRegistry, refEmitter, isCore, strictCtorDeps, semanticDepGraphUpdater,
1049+
injectableRegistry, refEmitter, referencesRegistry, isCore, strictCtorDeps, semanticDepGraphUpdater,
10501050
this.closureCompilerEnabled,
10511051
this.delegatingPerfRecorder,
10521052
) 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
@@ -5861,6 +5861,26 @@ function allTests(os: string) {
58615861
expect(ts.isClassDeclaration(id.parent)).toBe(true);
58625862
});
58635863

5864+
it('should report an error when a visible host directive is not exported', () => {
5865+
env.tsconfig({'flatModuleOutFile': 'flat.js'});
5866+
env.write('test.ts', `
5867+
import {Directive, NgModule} from '@angular/core';
5868+
5869+
@Directive({
5870+
standalone: true,
5871+
})
5872+
class HostDir {}
5873+
5874+
// The directive is not exported.
5875+
@Directive({selector: 'test', hostDirectives: [HostDir]})
5876+
export class Dir {}
5877+
`);
5878+
5879+
const errors = env.driveDiagnostics();
5880+
expect(errors.length).toBe(1);
5881+
expect(errors[0].code).toBe(ngErrorCode(ErrorCode.SYMBOL_NOT_EXPORTED));
5882+
});
5883+
58645884
it('should report an error when a deeply visible directive is not exported', () => {
58655885
env.tsconfig({'flatModuleOutFile': 'flat.js'});
58665886
env.write('test.ts', `

0 commit comments

Comments
 (0)