From 7f98b87ca0f6f0c99cc8d6c0ae09b6c5d8f9c483 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 4 Jun 2020 08:43:04 +0100 Subject: [PATCH] fix(ngcc): ensure that more dependencies are found by `EsmDependencyHost` (#37075) Previously this host was skipping files if they had imports that spanned multiple lines, or if the import was a dynamic import expression. PR Close #37075 --- .../src/dependencies/esm_dependency_host.ts | 2 +- .../dependencies/esm_dependency_host_spec.ts | 30 +++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts b/packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts index 2fc6d1bbd4..bbb4ef5156 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts @@ -93,7 +93,7 @@ export class EsmDependencyHost extends DependencyHostBase { * in this file, true otherwise. */ export function hasImportOrReexportStatements(source: string): boolean { - return /(import|export)\s.+from/.test(source); + return /(?:import|export)[\s\S]+?(["'])(?:(?:\\\1|.)*?)\1/.test(source); } diff --git a/packages/compiler-cli/ngcc/test/dependencies/esm_dependency_host_spec.ts b/packages/compiler-cli/ngcc/test/dependencies/esm_dependency_host_spec.ts index 4024b37c03..1932167c07 100644 --- a/packages/compiler-cli/ngcc/test/dependencies/esm_dependency_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/dependencies/esm_dependency_host_spec.ts @@ -180,13 +180,13 @@ runInEachFileSystem(() => { {name: _('/no/imports/or/re-exports/index.metadata.json'), contents: 'MOCK METADATA'}, { name: _('/external/imports/index.js'), - contents: `import {X} from 'lib-1';\nimport {Y} from 'lib-1/sub-1';` + contents: `import {\n X\n} from 'lib-1';\nimport {Y, Z} from 'lib-1/sub-1';` }, {name: _('/external/imports/package.json'), contents: '{"esm2015": "./index.js"}'}, {name: _('/external/imports/index.metadata.json'), contents: 'MOCK METADATA'}, { name: _('/external/re-exports/index.js'), - contents: `export {X} from 'lib-1';\nexport {Y} from 'lib-1/sub-1';` + contents: `export {X} from 'lib-1';\nexport {\n Y,\n Z\n} from 'lib-1/sub-1';` }, {name: _('/external/re-exports/package.json'), contents: '{"esm2015": "./index.js"}'}, {name: _('/external/re-exports/index.metadata.json'), contents: 'MOCK METADATA'}, @@ -288,20 +288,44 @@ runInEachFileSystem(() => { describe('hasImportOrReexportStatements', () => { it('should return true if there is an import statement', () => { expect(hasImportOrReexportStatements('import {X} from "some/x";')).toBe(true); + expect(hasImportOrReexportStatements('import {X} from \'some/x\';')).toBe(true); expect(hasImportOrReexportStatements('import * as X from "some/x";')).toBe(true); + expect(hasImportOrReexportStatements('import * as X from \'some/x\';')).toBe(true); expect(hasImportOrReexportStatements('blah blah\n\n import {X} from "some/x";\nblah blah')) .toBe(true); + expect( + hasImportOrReexportStatements('blah blah\n\n import {X} from \'some/x\';\nblah blah')) + .toBe(true); expect(hasImportOrReexportStatements('\t\timport {X} from "some/x";')).toBe(true); + expect(hasImportOrReexportStatements('\t\timport {X} from \'some/x\';')).toBe(true); + expect(hasImportOrReexportStatements('\t\timport {\n X,\n Y\n} from "some/x";')) + .toBe(true); + expect(hasImportOrReexportStatements('\t\timport {\n X,\n Y\n} from \'some/x\';')) + .toBe(true); + expect(hasImportOrReexportStatements('\t\timport "some/x";')).toBe(true); + expect(hasImportOrReexportStatements('\t\timport \'some/x\';')).toBe(true); }); + it('should return true if there is a re-export statement', () => { expect(hasImportOrReexportStatements('export {X} from "some/x";')).toBe(true); + expect(hasImportOrReexportStatements('export {X} from \'some/x\';')).toBe(true); expect(hasImportOrReexportStatements('blah blah\n\n export {X} from "some/x";\nblah blah')) .toBe(true); + expect( + hasImportOrReexportStatements('blah blah\n\n export {X} from \'some/x\';\nblah blah')) + .toBe(true); expect(hasImportOrReexportStatements('\t\texport {X} from "some/x";')).toBe(true); + expect(hasImportOrReexportStatements('\t\texport {X} from \'some/x\';')).toBe(true); + expect(hasImportOrReexportStatements('export {\n X,\n Y\n} from "some/x";')).toBe(true); + expect(hasImportOrReexportStatements('export {\n X,\n Y\n} from \'some/x\';')).toBe(true); expect(hasImportOrReexportStatements( - 'blah blah\n\n export * from "@angular/core;\nblah blah')) + 'blah blah\n\n export * from "@angular/core";\nblah blah')) + .toBe(true); + expect(hasImportOrReexportStatements( + 'blah blah\n\n export * from \'@angular/core\';\nblah blah')) .toBe(true); }); + it('should return false if there is no import nor re-export statement', () => { expect(hasImportOrReexportStatements('blah blah')).toBe(false); expect(hasImportOrReexportStatements('export function moo() {}')).toBe(false);