From 52544ffaa32c7315b8bedfe875046d893141b99f Mon Sep 17 00:00:00 2001 From: JoostK Date: Tue, 4 Dec 2018 22:10:37 +0100 Subject: [PATCH] fix(ivy): ngcc generates setClassMetadata calls for ES5 bundles (#27438) ngcc would feed ngtsc with the function declaration inside of an IIFE as that is considered the class symbol's declaration node, according to TypeScript's `ts.Symbol.valueDeclaration`. ngtsc however only considered variable decls and actual class decls as potential class declarations, so given the function declaration node it would fail to generate the `setClassMetadata` call. ngtsc no longer makes its own assumptions about what classes look like, but always asks the reflection host to yield this kind of information. PR Close #27438 --- packages/compiler-cli/src/ngcc/src/host/esm5_host.ts | 4 +++- .../compiler-cli/src/ngtsc/annotations/src/metadata.ts | 5 +---- packages/compiler-cli/src/ngtsc/host/src/reflection.ts | 2 +- packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts | 2 +- packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts | 8 ++------ 5 files changed, 8 insertions(+), 13 deletions(-) diff --git a/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts b/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts index 08a97914b5..8eb0837607 100644 --- a/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts @@ -38,7 +38,9 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost { /** * Check whether the given node actually represents a class. */ - isClass(node: ts.Node): boolean { return super.isClass(node) || !!this.getClassSymbol(node); } + isClass(node: ts.Node): node is ts.NamedDeclaration { + return super.isClass(node) || !!this.getClassSymbol(node); + } /** * Find a symbol for a node that we think is a class. diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts b/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts index c58b857603..79d18220c4 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts @@ -20,10 +20,7 @@ import {CtorParameter, Decorator, ReflectionHost} from '../../host'; */ export function generateSetClassMetadataCall( clazz: ts.Declaration, reflection: ReflectionHost, isCore: boolean): Statement|null { - // Classes come in two flavors, class declarations (ES2015) and variable declarations (ES5). - // Both must have a declared name to have metadata set on them. - if ((!ts.isClassDeclaration(clazz) && !ts.isVariableDeclaration(clazz)) || - clazz.name === undefined || !ts.isIdentifier(clazz.name)) { + if (!reflection.isClass(clazz) || clazz.name === undefined || !ts.isIdentifier(clazz.name)) { return null; } const id = ts.updateIdentifier(clazz.name); diff --git a/packages/compiler-cli/src/ngtsc/host/src/reflection.ts b/packages/compiler-cli/src/ngtsc/host/src/reflection.ts index 75bf27b71d..f448507a26 100644 --- a/packages/compiler-cli/src/ngtsc/host/src/reflection.ts +++ b/packages/compiler-cli/src/ngtsc/host/src/reflection.ts @@ -423,7 +423,7 @@ export interface ReflectionHost { /** * Check whether the given node actually represents a class. */ - isClass(node: ts.Node): boolean; + isClass(node: ts.Node): node is ts.NamedDeclaration; hasBaseClass(node: ts.Declaration): boolean; diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts b/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts index fffd71f355..28ab101980 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts @@ -145,7 +145,7 @@ export class TypeScriptReflectionHost implements ReflectionHost { return map; } - isClass(node: ts.Node): boolean { + isClass(node: ts.Node): node is ts.NamedDeclaration { // In TypeScript code, classes are ts.ClassDeclarations. return ts.isClassDeclaration(node); } diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts b/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts index 8223ed59b4..c323160fd9 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts @@ -353,7 +353,7 @@ class StaticInterpreter { return this.visitExpression(node.expression, context); } else if (ts.isNonNullExpression(node)) { return this.visitExpression(node.expression, context); - } else if (isPossibleClassDeclaration(node) && this.host.isClass(node)) { + } else if (this.host.isClass(node)) { return this.visitDeclaration(node, context); } else { return DYNAMIC_VALUE; @@ -565,7 +565,7 @@ class StaticInterpreter { return lhs[rhs]; } else if (lhs instanceof Reference) { const ref = lhs.node; - if (isPossibleClassDeclaration(ref) && this.host.isClass(ref)) { + if (this.host.isClass(ref)) { let absoluteModuleName = context.absoluteModuleName; if (lhs instanceof NodeReference || lhs instanceof AbsoluteReference) { absoluteModuleName = lhs.moduleName || absoluteModuleName; @@ -761,10 +761,6 @@ function identifierOfDeclaration(decl: ts.Declaration): ts.Identifier|undefined } } -function isPossibleClassDeclaration(node: ts.Node): node is ts.Declaration { - return ts.isClassDeclaration(node) || ts.isVariableDeclaration(node); -} - function isVariableDeclarationDeclared(node: ts.VariableDeclaration): boolean { if (node.parent === undefined || !ts.isVariableDeclarationList(node.parent)) { return false;