From 80f290e301b8c961ef488d1c230126dfc7680069 Mon Sep 17 00:00:00 2001 From: JoostK Date: Fri, 19 Jul 2019 23:22:52 +0200 Subject: [PATCH] fix(ivy): ngcc - recognize suffixed tslib helpers (#31614) An identifier may become repeated when bundling multiple source files into a single bundle, so bundlers have a strategy of suffixing non-unique identifiers with a suffix like $2. Since ngcc operates on such bundles, it needs to process potentially suffixed identifiers in their canonical form without the suffix. The "ngx-pagination" package was previously not compiled fully, as most decorators were not recognized. This commit ensures that identifiers are first canonicalized by removing the suffix, such that they are properly recognized and processed by ngcc. Fixes #31540 PR Close #31614 --- .../ngcc/src/host/esm2015_host.ts | 18 +++++--------- .../compiler-cli/ngcc/src/host/esm5_host.ts | 6 +++-- packages/compiler-cli/ngcc/src/utils.ts | 11 +++++++++ .../compiler-cli/ngcc/test/helpers/utils.ts | 18 +++++++------- .../host/esm2015_host_import_helper_spec.ts | 4 +++- .../test/host/esm5_host_import_helper_spec.ts | 4 +++- .../ngcc/test/host/esm5_host_spec.ts | 24 +++++++++++++++++++ 7 files changed, 60 insertions(+), 25 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/host/esm2015_host.ts b/packages/compiler-cli/ngcc/src/host/esm2015_host.ts index 41bc67ba50..24de49a39c 100644 --- a/packages/compiler-cli/ngcc/src/host/esm2015_host.ts +++ b/packages/compiler-cli/ngcc/src/host/esm2015_host.ts @@ -12,7 +12,7 @@ import {AbsoluteFsPath} from '../../../src/ngtsc/file_system'; import {ClassDeclaration, ClassMember, ClassMemberKind, ClassSymbol, CtorParameter, Declaration, Decorator, TypeScriptReflectionHost, isDecoratorIdentifier, reflectObjectLiteral} from '../../../src/ngtsc/reflection'; import {Logger} from '../logging/logger'; import {BundleProgram} from '../packages/bundle_program'; -import {findAll, getNameText, hasNameIdentifier, isDefined} from '../utils'; +import {findAll, getNameText, hasNameIdentifier, isDefined, stripDollarSuffix} from '../utils'; import {ModuleWithProvidersFunction, NgccReflectionHost, PRE_R3_MARKER, SwitchableVariableDeclaration, isSwitchableVariableDeclaration} from './ngcc_host'; @@ -889,14 +889,8 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N } const call = expression; - const helperCallFn = - ts.isPropertyAccessExpression(call.expression) ? call.expression.name : call.expression; - if (!ts.isIdentifier(helperCallFn)) { - return null; - } - - const name = helperCallFn.text; - if (name === '__metadata') { + const helperName = getCalleeName(call); + if (helperName === '__metadata') { // This is a `tslib.__metadata` call, reflect to arguments into a `ParameterTypes` object // if the metadata key is "design:paramtypes". const key = call.arguments[0]; @@ -915,7 +909,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N }; } - if (name === '__param') { + if (helperName === '__param') { // This is a `tslib.__param` call that is reflected into a `ParameterDecorators` object. const indexArg = call.arguments[0]; const index = indexArg && ts.isNumericLiteral(indexArg) ? parseInt(indexArg.text, 10) : NaN; @@ -1613,10 +1607,10 @@ export function getPropertyValueFromSymbol(propSymbol: ts.Symbol): ts.Expression */ function getCalleeName(call: ts.CallExpression): string|null { if (ts.isIdentifier(call.expression)) { - return call.expression.text; + return stripDollarSuffix(call.expression.text); } if (ts.isPropertyAccessExpression(call.expression)) { - return call.expression.name.text; + return stripDollarSuffix(call.expression.name.text); } return null; } diff --git a/packages/compiler-cli/ngcc/src/host/esm5_host.ts b/packages/compiler-cli/ngcc/src/host/esm5_host.ts index a18a30bd4e..723db3dc79 100644 --- a/packages/compiler-cli/ngcc/src/host/esm5_host.ts +++ b/packages/compiler-cli/ngcc/src/host/esm5_host.ts @@ -10,7 +10,7 @@ import * as ts from 'typescript'; import {ClassDeclaration, ClassMember, ClassMemberKind, ClassSymbol, CtorParameter, Declaration, Decorator, FunctionDefinition, Parameter, TsHelperFn, isNamedVariableDeclaration, reflectObjectLiteral} from '../../../src/ngtsc/reflection'; import {isFromDtsFile} from '../../../src/ngtsc/util/src/typescript'; -import {getNameText, hasNameIdentifier} from '../utils'; +import {getNameText, hasNameIdentifier, stripDollarSuffix} from '../utils'; import {Esm2015ReflectionHost, ParamInfo, getPropertyValueFromSymbol, isAssignmentStatement} from './esm2015_host'; @@ -659,7 +659,9 @@ function reflectArrayElement(element: ts.Expression) { * helper. */ function getTsHelperFn(node: ts.NamedDeclaration): TsHelperFn|null { - const name = node.name !== undefined && ts.isIdentifier(node.name) && node.name.text; + const name = node.name !== undefined && ts.isIdentifier(node.name) ? + stripDollarSuffix(node.name.text) : + null; if (name === '__spread') { return TsHelperFn.Spread; diff --git a/packages/compiler-cli/ngcc/src/utils.ts b/packages/compiler-cli/ngcc/src/utils.ts index c3eb08378b..1b221fdde3 100644 --- a/packages/compiler-cli/ngcc/src/utils.ts +++ b/packages/compiler-cli/ngcc/src/utils.ts @@ -82,3 +82,14 @@ export function resolveFileWithPostfixes( } return null; } + +/** + * An identifier may become repeated when bundling multiple source files into a single bundle, so + * bundlers have a strategy of suffixing non-unique identifiers with a suffix like $2. This function + * strips off such suffixes, so that ngcc deals with the canonical name of an identifier. + * @param value The value to strip any suffix of, if applicable. + * @returns The canonical representation of the value, without any suffix. + */ +export function stripDollarSuffix(value: string): string { + return value.replace(/\$\d+$/, ''); +} diff --git a/packages/compiler-cli/ngcc/test/helpers/utils.ts b/packages/compiler-cli/ngcc/test/helpers/utils.ts index bfb679ce86..72ef2779c8 100644 --- a/packages/compiler-cli/ngcc/test/helpers/utils.ts +++ b/packages/compiler-cli/ngcc/test/helpers/utils.ts @@ -78,18 +78,18 @@ export function convertToDirectTsLibImport(filesystem: TestFile[]) { }); } -export function convertToInlineTsLib(filesystem: TestFile[]) { +export function convertToInlineTsLib(filesystem: TestFile[], suffix: string = '') { return filesystem.map(file => { const contents = file.contents .replace(`import * as tslib_1 from 'tslib';`, ` -var __decorate = null; -var __metadata = null; -var __read = null; -var __values = null; -var __param = null; -var __extends = null; -var __assign = null; -`).replace(/tslib_1\./g, ''); +var __decorate${suffix} = null; +var __metadata${suffix} = null; +var __read${suffix} = null; +var __values${suffix} = null; +var __param${suffix} = null; +var __extends${suffix} = null; +var __assign${suffix} = null; +`).replace(/tslib_1\.([_a-z]+)/gi, '$1' + suffix.replace('$', '$$')); return {...file, contents}; }); } diff --git a/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts b/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts index 2eb69d28e5..ab46d1b66c 100644 --- a/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts @@ -112,15 +112,17 @@ runInEachFileSystem(() => { const DIRECT_IMPORT_FILES = convertToDirectTsLibImport(NAMESPACED_IMPORT_FILES); const INLINE_FILES = convertToInlineTsLib(NAMESPACED_IMPORT_FILES); + const INLINE_SUFFIXED_FILES = convertToInlineTsLib(NAMESPACED_IMPORT_FILES, '$2'); FILES = { 'namespaced': NAMESPACED_IMPORT_FILES, 'direct import': DIRECT_IMPORT_FILES, 'inline': INLINE_FILES, + 'inline suffixed': INLINE_SUFFIXED_FILES, }; }); - ['namespaced', 'direct import', 'inline'].forEach(label => { + ['namespaced', 'direct import', 'inline', 'inline suffixed'].forEach(label => { describe(`[${label}]`, () => { beforeEach(() => { const fs = getFileSystem(); diff --git a/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts b/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts index be6eb5a975..10a16f88c3 100644 --- a/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts @@ -133,15 +133,17 @@ export { SomeDirective }; const DIRECT_IMPORT_FILES = convertToDirectTsLibImport(NAMESPACED_IMPORT_FILES); const INLINE_FILES = convertToInlineTsLib(NAMESPACED_IMPORT_FILES); + const INLINE_SUFFIXED_FILES = convertToInlineTsLib(NAMESPACED_IMPORT_FILES, '$2'); FILES = { 'namespaced': NAMESPACED_IMPORT_FILES, 'direct import': DIRECT_IMPORT_FILES, 'inline': INLINE_FILES, + 'inline suffixed': INLINE_SUFFIXED_FILES, }; }); - ['namespaced', 'direct import', 'inline'].forEach(label => { + ['namespaced', 'direct import', 'inline', 'inline suffixed'].forEach(label => { describe(`[${label}]`, () => { beforeEach(() => { const fs = getFileSystem(); diff --git a/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts b/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts index 11ff1a063e..e475213324 100644 --- a/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts @@ -1669,6 +1669,30 @@ runInEachFileSystem(() => { expect(definition.helper).toBe(TsHelperFn.Spread); expect(definition.parameters.length).toEqual(0); }); + + it('should recognize TypeScript __spread helper function implementation when suffixed', + () => { + const file: TestFile = { + name: _('/implementation.js'), + contents: ` + var __spread$2 = (this && this.__spread$2) || function () { + for (var ar = [], i = 0; i < arguments.length; i++) ar = ar.concat(__read(arguments[i])); + return ar; + };`, + }; + loadTestFiles([file]); + const {program} = makeTestBundleProgram(file.name); + const host = new Esm5ReflectionHost(new MockLogger(), false, program.getTypeChecker()); + + const node = + getDeclaration(program, file.name, '__spread$2', ts.isVariableDeclaration) !; + + const definition = host.getDefinitionOfFunction(node) !; + expect(definition.node).toBe(node); + expect(definition.body).toBeNull(); + expect(definition.helper).toBe(TsHelperFn.Spread); + expect(definition.parameters.length).toEqual(0); + }); }); describe('getImportOfIdentifier()', () => {