cargdev/angular
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(compiler-cli): adding references to const enums in runtime code (#38542)

Browse Source 
We had a couple of places where we were assuming that if a particular
symbol has a value, then it will exist at runtime. This is true in most cases,
but it breaks down for `const` enums.

Fixes #38513.

PR Close #38542
This commit is contained in:
crisbeto 2020-08-20 23:21:21 +02:00 committed by Misko Hevery
parent 2a643e1ab6
commit e7da4040d6
4 changed files with 105 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts
View File

@ -35,8 +35,9 @@ export function typeToValue(
const {local, decl} = symbols;
// It's only valid to convert a type reference to a value reference if the type actually
// has a value declaration associated with it.
if (decl.valueDeclaration === undefined) {
// has a value declaration associated with it. Note that const enums are an exception,
// because while they do have a value declaration, they don't exist at runtime.
if (decl.valueDeclaration === undefined || decl.flags & ts.SymbolFlags.ConstEnum) {
return noValueDeclaration(typeNode, decl.declarations[0]);
}

15
packages/compiler-cli/src/transformers/downlevel_decorators_transform.ts
View File

@ -298,15 +298,20 @@ function typeReferenceToExpression(
}
/**
* Returns true if the given symbol refers to a value (as distinct from a type).
* Checks whether a given symbol refers to a value that exists at runtime (as distinct from a type).
*
* Expands aliases, which is important for the case where
* import * as x from 'some-module';
* and x is now a value (the module object).
*/
function symbolIsValue(tc: ts.TypeChecker, sym: ts.Symbol): boolean {
if (sym.flags & ts.SymbolFlags.Alias) sym = tc.getAliasedSymbol(sym);
return (sym.flags & ts.SymbolFlags.Value) !== 0;
function symbolIsRuntimeValue(typeChecker: ts.TypeChecker, symbol: ts.Symbol): boolean {
if (symbol.flags & ts.SymbolFlags.Alias) {
symbol = typeChecker.getAliasedSymbol(symbol);
}
// Note that const enums are a special case, because
// while they have a value, they don't exist at runtime.
return (symbol.flags & ts.SymbolFlags.Value & ts.SymbolFlags.ConstEnumExcludes) !== 0;
}
/** ParameterDecorationInfo describes the information for a single constructor parameter. */
@ -351,7 +356,7 @@ export function getDownlevelDecoratorsTransform(
const symbol = typeChecker.getSymbolAtLocation(name);
// Check if the entity name references a symbol that is an actual value. If it is not, it
// cannot be referenced by an expression, so return undefined.
if (!symbol || !symbolIsValue(typeChecker, symbol) || !symbol.declarations ||
if (!symbol || !symbolIsRuntimeValue(typeChecker, symbol) || !symbol.declarations ||
symbol.declarations.length === 0) {
return undefined;
}

47
packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
View File

@ -4369,6 +4369,53 @@ runInEachFileSystem(os => {
expect(jsContents).toMatch(setClassMetadataRegExp('type: undefined'));
});
it('should use `undefined` in setClassMetadata for const enums', () => {
env.write(`keycodes.ts`, `
export const enum KeyCodes {A, B};
`);
env.write(`test.ts`, `
import {Component, Inject} from '@angular/core';
import {KeyCodes} from './keycodes';
@Component({
selector: 'some-comp',
template: '...',
})
export class SomeComp {
constructor(@Inject('arg-token') arg: KeyCodes) {}
}
`);
env.driveMain();
const jsContents = trim(env.getContents('test.js'));
expect(jsContents).not.toContain(`import { KeyCodes } from './keycodes';`);
// Note: `type: undefined` below, since KeyCodes can't be represented as a value
expect(jsContents).toMatch(setClassMetadataRegExp('type: undefined'));
});
it('should preserve the types of non-const enums in setClassMetadata', () => {
env.write(`keycodes.ts`, `
export enum KeyCodes {A, B};
`);
env.write(`test.ts`, `
import {Component, Inject} from '@angular/core';
import {KeyCodes} from './keycodes';
@Component({
selector: 'some-comp',
template: '...',
})
export class SomeComp {
constructor(@Inject('arg-token') arg: KeyCodes) {}
}
`);
env.driveMain();
const jsContents = trim(env.getContents('test.js'));
expect(jsContents).toContain(`import { KeyCodes } from './keycodes';`);
expect(jsContents).toMatch(setClassMetadataRegExp('type: i1.KeyCodes'));
});
it('should use `undefined` in setClassMetadata if types originate from type-only imports',
() => {
env.write(`types.ts`, `

64
packages/compiler-cli/test/transformers/downlevel_decorators_transform_spec.ts
View File

@ -192,7 +192,7 @@ describe('downlevel decorator transform', () => {
it('should downlevel Angular-decorated class member', () => {
const {output} = transform(`
import {Input} from '@angular/core';
export class MyDir {
@Input() disabled: boolean = false;
}
@ -231,7 +231,7 @@ describe('downlevel decorator transform', () => {
const {output} = transform(`
import {Input} from '@angular/core';
import {MyOtherClass} from './other-file';
export class MyDir {
@Input() trigger: HTMLElement;
@Input() fromOtherFile: MyOtherClass;
@ -255,7 +255,7 @@ describe('downlevel decorator transform', () => {
`
import {Directive} from '@angular/core';
import {MyOtherClass} from './other-file';
@Directive()
export class MyDir {
constructor(other: MyOtherClass) {}
@ -281,7 +281,7 @@ describe('downlevel decorator transform', () => {
`
import {Directive} from '@angular/core';
import {MyOtherClass} from './other-file';
@Directive()
export class MyDir {
constructor(other: MyOtherClass) {}
@ -307,7 +307,7 @@ describe('downlevel decorator transform', () => {
const {output} = transform(`
import {Directive} from '@angular/core';
import * as externalFile from './other-file';
@Directive()
export class MyDir {
constructor(other: externalFile.MyOtherClass) {}
@ -329,11 +329,11 @@ describe('downlevel decorator transform', () => {
it('should properly serialize constructor parameter with local qualified name type', () => {
const {output} = transform(`
import {Directive} from '@angular/core';
namespace other {
export class OtherClass {}
};
@Directive()
export class MyDir {
constructor(other: other.OtherClass) {}
@ -355,7 +355,7 @@ describe('downlevel decorator transform', () => {
it('should properly downlevel constructor parameter decorators', () => {
const {output} = transform(`
import {Inject, Directive, DOCUMENT} from '@angular/core';
@Directive()
export class MyDir {
constructor(@Inject(DOCUMENT) document: Document) {}
@ -376,7 +376,7 @@ describe('downlevel decorator transform', () => {
it('should properly downlevel constructor parameters with union type', () => {
const {output} = transform(`
import {Optional, Directive, NgZone} from '@angular/core';
@Directive()
export class MyDir {
constructor(@Optional() ngZone: NgZone|null) {}
@ -546,18 +546,20 @@ describe('downlevel decorator transform', () => {
export default interface {
hello: false;
}
export const enum KeyCodes {A, B}
`);
const {output} = transform(`
import {Directive, Inject} from '@angular/core';
import * as angular from './external';
import {IOverlay} from './external';
import {IOverlay, KeyCodes} from './external';
import TypeFromDefaultImport from './external';
@Directive()
export class MyDir {
constructor(@Inject('$state') param: angular.IState,
@Inject('$overlay') other: IOverlay,
@Inject('$default') default: TypeFromDefaultImport) {}
@Inject('$default') default: TypeFromDefaultImport,
@Inject('$keyCodes') keyCodes: KeyCodes) {}
}
`);
@ -570,7 +572,8 @@ describe('downlevel decorator transform', () => {
MyDir.ctorParameters = () => [
{ type: undefined, decorators: [{ type: core_1.Inject, args: ['$state',] }] },
{ type: undefined, decorators: [{ type: core_1.Inject, args: ['$overlay',] }] },
{ type: undefined, decorators: [{ type: core_1.Inject, args: ['$default',] }] }
{ type: undefined, decorators: [{ type: core_1.Inject, args: ['$default',] }] },
{ type: undefined, decorators: [{ type: core_1.Inject, args: ['$keyCodes',] }] }
];
`);
});
@ -593,7 +596,7 @@ describe('downlevel decorator transform', () => {
const {output} = transform(
`
import {Directive} from '@angular/core';
export class MyInjectedClass {}
@Directive()
@ -609,13 +612,36 @@ describe('downlevel decorator transform', () => {
expect(output).not.toContain('tslib');
});
it('should capture a non-const enum used as a constructor type', () => {
const {output} = transform(`
import {Component} from '@angular/core';
export enum Values {A, B};
@Component({template: 'hello'})
export class MyComp {
constructor(v: Values) {}
}
`);
expect(diagnostics.length).toBe(0);
expect(output).toContain(dedent`
MyComp.decorators = [
{ type: core_1.Component, args: [{ template: 'hello' },] }
];
MyComp.ctorParameters = () => [
{ type: Values }
];`);
expect(output).not.toContain('tslib');
});
describe('class decorators skipped', () => {
beforeEach(() => skipClassDecorators = true);
it('should not downlevel Angular class decorators', () => {
const {output} = transform(`
import {Injectable} from '@angular/core';
@Injectable()
export class MyService {}
`);
@ -632,10 +658,10 @@ describe('downlevel decorator transform', () => {
it('should downlevel constructor parameters', () => {
const {output} = transform(`
import {Injectable} from '@angular/core';
@Injectable()
export class InjectClass {}
@Injectable()
export class MyService {
constructor(dep: InjectClass) {}
@ -658,10 +684,10 @@ describe('downlevel decorator transform', () => {
it('should downlevel constructor parameter decorators', () => {
const {output} = transform(`
import {Injectable, Inject} from '@angular/core';
@Injectable()
export class InjectClass {}
@Injectable()
export class MyService {
constructor(@Inject('test') dep: InjectClass) {}
@ -684,7 +710,7 @@ describe('downlevel decorator transform', () => {
it('should downlevel class member Angular decorators', () => {
const {output} = transform(`
import {Injectable, Input} from '@angular/core';
export class MyService {
@Input() disabled: boolean;
}