fix(ngcc): properly detect origin of constructor param types (#33901)
The ReflectionHost supports enumeration of constructor parameters, and one piece of information it returns describes the origin of the parameter's type. Parameter types come in two flavors: local (the type is not imported from anywhere) or non-local (the type comes via an import). ngcc incorrectly classified all type parameters as 'local', because in the source files that ngcc processes the type parameter is a real ts.Identifer. However, that identifier may still have come from an import and thus might be non-local. This commit changes ngcc's ReflectionHost(s) to properly recognize and report these non-local type references. Fixes #33677 PR Close #33901
This commit is contained in:
parent
c68200cf42
commit
29b8666b10
|
@ -9,7 +9,7 @@
|
|||
import * as ts from 'typescript';
|
||||
|
||||
import {AbsoluteFsPath} from '../../../src/ngtsc/file_system';
|
||||
import {ClassDeclaration, ClassMember, ClassMemberKind, ConcreteDeclaration, CtorParameter, Declaration, Decorator, TypeScriptReflectionHost, isDecoratorIdentifier, reflectObjectLiteral} from '../../../src/ngtsc/reflection';
|
||||
import {ClassDeclaration, ClassMember, ClassMemberKind, ConcreteDeclaration, CtorParameter, Declaration, Decorator, TypeScriptReflectionHost, TypeValueReference, isDecoratorIdentifier, reflectObjectLiteral} from '../../../src/ngtsc/reflection';
|
||||
import {isWithinPackage} from '../analysis/util';
|
||||
import {Logger} from '../logging/logger';
|
||||
import {BundleProgram} from '../packages/bundle_program';
|
||||
|
@ -1357,12 +1357,34 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
|
|||
constructorParamInfo[index] :
|
||||
{decorators: null, typeExpression: null};
|
||||
const nameNode = node.name;
|
||||
|
||||
let typeValueReference: TypeValueReference|null = null;
|
||||
if (typeExpression !== null) {
|
||||
// `typeExpression` is an expression in a "type" context. Resolve it to a declared value.
|
||||
// Either it's a reference to an imported type, or a type declared locally. Distinguish the
|
||||
// two cases with `getDeclarationOfExpression`.
|
||||
const decl = this.getDeclarationOfExpression(typeExpression);
|
||||
if (decl !== null && decl.node !== null && decl.viaModule !== null &&
|
||||
isNamedDeclaration(decl.node)) {
|
||||
typeValueReference = {
|
||||
local: false,
|
||||
valueDeclaration: decl.node,
|
||||
moduleName: decl.viaModule,
|
||||
name: decl.node.name.text,
|
||||
};
|
||||
} else {
|
||||
typeValueReference = {
|
||||
local: true,
|
||||
expression: typeExpression,
|
||||
defaultImportStatement: null,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
name: getNameText(nameNode),
|
||||
nameNode,
|
||||
typeValueReference: typeExpression !== null ?
|
||||
{local: true as true, expression: typeExpression, defaultImportStatement: null} :
|
||||
null,
|
||||
typeValueReference,
|
||||
typeNode: null, decorators
|
||||
};
|
||||
});
|
||||
|
|
|
@ -54,11 +54,10 @@ runInEachFileSystem(() => {
|
|||
SOME_DIRECTIVE_FILE = {
|
||||
name: _('/some_directive.js'),
|
||||
contents: `
|
||||
import { Directive, Inject, InjectionToken, Input, HostListener, HostBinding } from '@angular/core';
|
||||
import { Directive, Inject, InjectionToken, Input, HostListener, HostBinding, ViewContainerRef, TemplateRef } from '@angular/core';
|
||||
|
||||
const INJECTED_TOKEN = new InjectionToken('injected');
|
||||
const ViewContainerRef = {};
|
||||
const TemplateRef = {};
|
||||
const TestToken = {};
|
||||
|
||||
class SomeDirective {
|
||||
constructor(_viewContainer, _template, injected) {
|
||||
|
@ -196,6 +195,7 @@ runInEachFileSystem(() => {
|
|||
class NoArgsProperty {
|
||||
}
|
||||
NoArgsProperty.decorators = [
|
||||
|
||||
{ type: Directive },
|
||||
];
|
||||
|
||||
|
@ -1099,6 +1099,7 @@ runInEachFileSystem(() => {
|
|||
|
||||
describe('getConstructorParameters()', () => {
|
||||
it('should find the decorated constructor parameters', () => {
|
||||
loadFakeCore(getFileSystem());
|
||||
loadTestFiles([SOME_DIRECTIVE_FILE]);
|
||||
const {program} = makeTestBundleProgram(SOME_DIRECTIVE_FILE.name);
|
||||
const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker());
|
||||
|
@ -1111,7 +1112,7 @@ runInEachFileSystem(() => {
|
|||
'_viewContainer', '_template', 'injected'
|
||||
]);
|
||||
expectTypeValueReferencesForParameters(
|
||||
parameters, ['ViewContainerRef', 'TemplateRef', null]);
|
||||
parameters, ['ViewContainerRef', 'TemplateRef', null], '@angular/core');
|
||||
});
|
||||
|
||||
it('should accept `ctorParameters` as an array', () => {
|
||||
|
@ -1499,23 +1500,15 @@ runInEachFileSystem(() => {
|
|||
|
||||
describe('getDeclarationOfIdentifier()', () => {
|
||||
it('should return the declaration of a locally defined identifier', () => {
|
||||
loadFakeCore(getFileSystem());
|
||||
loadTestFiles([SOME_DIRECTIVE_FILE]);
|
||||
const {program} = makeTestBundleProgram(SOME_DIRECTIVE_FILE.name);
|
||||
const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker());
|
||||
const classNode = getDeclaration(
|
||||
program, SOME_DIRECTIVE_FILE.name, 'SomeDirective', isNamedClassDeclaration);
|
||||
const ctrDecorators = host.getConstructorParameters(classNode) !;
|
||||
const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference !as{
|
||||
local: true,
|
||||
expression: ts.Identifier,
|
||||
defaultImportStatement: null,
|
||||
}).expression;
|
||||
|
||||
const expectedDeclarationNode = getDeclaration(
|
||||
program, SOME_DIRECTIVE_FILE.name, 'ViewContainerRef', isNamedVariableDeclaration);
|
||||
const actualDeclaration = host.getDeclarationOfIdentifier(identifierOfViewContainerRef);
|
||||
const actualDeclaration = host.getDeclarationOfIdentifier(classNode.name);
|
||||
expect(actualDeclaration).not.toBe(null);
|
||||
expect(actualDeclaration !.node).toBe(expectedDeclarationNode);
|
||||
expect(actualDeclaration !.node).toBe(classNode);
|
||||
expect(actualDeclaration !.viaModule).toBe(null);
|
||||
});
|
||||
|
||||
|
|
|
@ -1560,9 +1560,6 @@ runInEachFileSystem(() => {
|
|||
|
||||
expect(decorators.length).toEqual(1);
|
||||
expect(decorators[0].import).toBe(mockImportInfo);
|
||||
|
||||
const typeIdentifier = spy.calls.mostRecent().args[0] as ts.Identifier;
|
||||
expect(typeIdentifier.text).toBe('Inject');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
@ -14,15 +14,26 @@ import {CtorParameter} from '../../../src/ngtsc/reflection';
|
|||
* names.
|
||||
*/
|
||||
export function expectTypeValueReferencesForParameters(
|
||||
parameters: CtorParameter[], expectedParams: (string | null)[]) {
|
||||
parameters: CtorParameter[], expectedParams: (string | null)[],
|
||||
fromModule: string | null = null) {
|
||||
parameters !.forEach((param, idx) => {
|
||||
const expected = expectedParams[idx];
|
||||
if (expected !== null) {
|
||||
if (param.typeValueReference === null || !param.typeValueReference.local ||
|
||||
!ts.isIdentifier(param.typeValueReference.expression)) {
|
||||
if (param.typeValueReference === null) {
|
||||
fail(`Incorrect typeValueReference generated, expected ${expected}`);
|
||||
} else {
|
||||
expect(param.typeValueReference.expression.text).toEqual(expected);
|
||||
} else if (param.typeValueReference.local && fromModule !== null) {
|
||||
fail(`Incorrect typeValueReference generated, expected non-local`);
|
||||
} else if (!param.typeValueReference.local && fromModule === null) {
|
||||
fail(`Incorrect typeValueReference generated, expected local`);
|
||||
} else if (param.typeValueReference.local) {
|
||||
if (!ts.isIdentifier(param.typeValueReference.expression)) {
|
||||
fail(`Incorrect typeValueReference generated, expected identifer`);
|
||||
} else {
|
||||
expect(param.typeValueReference.expression.text).toEqual(expected);
|
||||
}
|
||||
} else if (param.typeValueReference !== null) {
|
||||
expect(param.typeValueReference.moduleName).toBe(fromModule !);
|
||||
expect(param.typeValueReference.name).toBe(expected);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
|
Loading…
Reference in New Issue