fix(ivy): avoid missing imports for types that can be represented as values (#28941)
Prior to this change, TypeScript stripped out some imports in case we reference a type that can be represented as a value (for ex. classes). This fix ensures that we use correct symbol identifier, which makes TypeScript retain the necessary import statements. PR Close #28941
This commit is contained in:
parent
91a161aa13
commit
772b24ccc3
|
@ -68,18 +68,32 @@ export class TypeScriptReflectionHost implements ReflectionHost {
|
||||||
}
|
}
|
||||||
|
|
||||||
// It's not possible to get a value expression if the parameter doesn't even have a type.
|
// It's not possible to get a value expression if the parameter doesn't even have a type.
|
||||||
if (typeNode) {
|
if (typeNode && ts.isTypeReferenceNode(typeNode)) {
|
||||||
// It's only valid to convert a type reference to a value reference if the type actually has
|
// It's only valid to convert a type reference to a value reference if the type actually has
|
||||||
// a value declaration associated with it.
|
// a value declaration associated with it.
|
||||||
let type: ts.Type|null = this.checker.getTypeFromTypeNode(typeNode);
|
let symbol: ts.Symbol|undefined = this.checker.getSymbolAtLocation(typeNode.typeName);
|
||||||
|
|
||||||
if (type && type.symbol !== undefined && type.symbol.valueDeclaration !== undefined) {
|
if (symbol !== undefined) {
|
||||||
|
let resolvedSymbol = symbol;
|
||||||
|
if (symbol.flags & ts.SymbolFlags.Alias) {
|
||||||
|
resolvedSymbol = this.checker.getAliasedSymbol(symbol);
|
||||||
|
}
|
||||||
|
if (resolvedSymbol.valueDeclaration !== undefined) {
|
||||||
// The type points to a valid value declaration. Rewrite the TypeReference into an
|
// The type points to a valid value declaration. Rewrite the TypeReference into an
|
||||||
// Expression
|
// Expression which references the value pointed to by the TypeReference, if possible.
|
||||||
// which references the value pointed to by the TypeReference, if possible.
|
const firstDecl = symbol.declarations && symbol.declarations[0];
|
||||||
|
if (firstDecl && ts.isImportSpecifier(firstDecl)) {
|
||||||
|
// Making sure TS produces the necessary imports in case a symbol was declared in a
|
||||||
|
// different script and imported. To do that we check symbol's first declaration and
|
||||||
|
// if it's an import - use its identifier. The `Identifier` from the `ImportSpecifier`
|
||||||
|
// knows it could be a value reference, and will emit as one if needed.
|
||||||
|
typeValueExpr = ts.updateIdentifier(firstDecl.name);
|
||||||
|
} else {
|
||||||
typeValueExpr = typeNodeToValueExpr(typeNode);
|
typeValueExpr = typeNodeToValueExpr(typeNode);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
name,
|
name,
|
||||||
|
|
|
@ -56,6 +56,7 @@ export class NgtscTestEnvironment {
|
||||||
|
|
||||||
env.write('tsconfig-base.json', `{
|
env.write('tsconfig-base.json', `{
|
||||||
"compilerOptions": {
|
"compilerOptions": {
|
||||||
|
"emitDecoratorMetadata": true,
|
||||||
"experimentalDecorators": true,
|
"experimentalDecorators": true,
|
||||||
"skipLibCheck": true,
|
"skipLibCheck": true,
|
||||||
"noImplicitAny": true,
|
"noImplicitAny": true,
|
||||||
|
|
|
@ -26,6 +26,9 @@ const contentQueryRegExp = (predicate: string, descend: boolean, ref?: string):
|
||||||
return new RegExp(`i0\\.ɵcontentQuery\\(dirIndex, ${predicate}, ${descend}, ${maybeRef}\\)`);
|
return new RegExp(`i0\\.ɵcontentQuery\\(dirIndex, ${predicate}, ${descend}, ${maybeRef}\\)`);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const setClassMetadataRegExp = (expectedType: string): RegExp =>
|
||||||
|
new RegExp(`setClassMetadata(.*?${expectedType}.*?)`);
|
||||||
|
|
||||||
describe('ngtsc behavioral tests', () => {
|
describe('ngtsc behavioral tests', () => {
|
||||||
let env !: NgtscTestEnvironment;
|
let env !: NgtscTestEnvironment;
|
||||||
|
|
||||||
|
@ -1843,6 +1846,97 @@ describe('ngtsc behavioral tests', () => {
|
||||||
expect(jsContents).toContain('ɵsetClassMetadata(TestPipe, ');
|
expect(jsContents).toContain('ɵsetClassMetadata(TestPipe, ');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should use imported types in setClassMetadata if they can be represented as values', () => {
|
||||||
|
env.tsconfig({});
|
||||||
|
|
||||||
|
env.write(`types.ts`, `
|
||||||
|
export class MyTypeA {}
|
||||||
|
export class MyTypeB {}
|
||||||
|
`);
|
||||||
|
env.write(`test.ts`, `
|
||||||
|
import {Component, Inject, Injectable} from '@angular/core';
|
||||||
|
import {MyTypeA, MyTypeB} from './types';
|
||||||
|
|
||||||
|
@Injectable({providedIn: 'root'})
|
||||||
|
export class SomeService {
|
||||||
|
constructor(arg: MyTypeA) {}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Component({
|
||||||
|
selector: 'some-comp',
|
||||||
|
template: '...',
|
||||||
|
})
|
||||||
|
export class SomeComp {
|
||||||
|
constructor(@Inject('arg-token') arg: MyTypeB) {}
|
||||||
|
}
|
||||||
|
`);
|
||||||
|
|
||||||
|
env.driveMain();
|
||||||
|
const jsContents = trim(env.getContents('test.js'));
|
||||||
|
expect(jsContents).toContain(`import { MyTypeA, MyTypeB } from './types';`);
|
||||||
|
expect(jsContents).toMatch(setClassMetadataRegExp('type: MyTypeA'));
|
||||||
|
expect(jsContents).toMatch(setClassMetadataRegExp('type: MyTypeB'));
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should use imported types in setClassMetadata if they can be represented as values and imported as `* as foo`',
|
||||||
|
() => {
|
||||||
|
env.tsconfig({});
|
||||||
|
|
||||||
|
env.write(`types.ts`, `
|
||||||
|
export class MyTypeA {}
|
||||||
|
export class MyTypeB {}
|
||||||
|
`);
|
||||||
|
env.write(`test.ts`, `
|
||||||
|
import {Component, Inject, Injectable} from '@angular/core';
|
||||||
|
import * as types from './types';
|
||||||
|
|
||||||
|
@Injectable({providedIn: 'root'})
|
||||||
|
export class SomeService {
|
||||||
|
constructor(arg: types.MyTypeA) {}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Component({
|
||||||
|
selector: 'some-comp',
|
||||||
|
template: '...',
|
||||||
|
})
|
||||||
|
export class SomeComp {
|
||||||
|
constructor(@Inject('arg-token') arg: types.MyTypeB) {}
|
||||||
|
}
|
||||||
|
`);
|
||||||
|
|
||||||
|
env.driveMain();
|
||||||
|
const jsContents = trim(env.getContents('test.js'));
|
||||||
|
expect(jsContents).toContain(`import * as types from './types';`);
|
||||||
|
expect(jsContents).toMatch(setClassMetadataRegExp('type: types.MyTypeA'));
|
||||||
|
expect(jsContents).toMatch(setClassMetadataRegExp('type: types.MyTypeB'));
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should use `undefined` in setClassMetadata if types can\'t be represented as values', () => {
|
||||||
|
env.tsconfig({});
|
||||||
|
|
||||||
|
env.write(`types.ts`, `
|
||||||
|
export type MyType = Map<any, any>;
|
||||||
|
`);
|
||||||
|
env.write(`test.ts`, `
|
||||||
|
import {Component, Inject, Injectable} from '@angular/core';
|
||||||
|
import {MyType} from './types';
|
||||||
|
|
||||||
|
@Component({
|
||||||
|
selector: 'some-comp',
|
||||||
|
template: '...',
|
||||||
|
})
|
||||||
|
export class SomeComp {
|
||||||
|
constructor(@Inject('arg-token') arg: MyType) {}
|
||||||
|
}
|
||||||
|
`);
|
||||||
|
|
||||||
|
env.driveMain();
|
||||||
|
const jsContents = trim(env.getContents('test.js'));
|
||||||
|
expect(jsContents).not.toContain(`import { MyType } from './types';`);
|
||||||
|
// Note: `type: undefined` below, since MyType can't be represented as a value
|
||||||
|
expect(jsContents).toMatch(setClassMetadataRegExp('type: undefined'));
|
||||||
|
});
|
||||||
|
|
||||||
it('should not throw in case whitespaces and HTML comments are present inside <ng-content>',
|
it('should not throw in case whitespaces and HTML comments are present inside <ng-content>',
|
||||||
() => {
|
() => {
|
||||||
env.tsconfig();
|
env.tsconfig();
|
||||||
|
|
Loading…
Reference in New Issue