From 6af9b8fb929423e95897721978705f17ea69f1b4 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 24 Apr 2019 14:47:50 +0100 Subject: [PATCH] fix(ivy): ngcc - do not copy external files when writing bundles (#30085) Only the JS files that are actually part of the entry-point should be copied to the new entry-point folder in the `NewEntryPointFileWriter`. Previously some typings and external JS files were being copied which was messing up the node_modules structure. Fixes https://github.com/angular/angular-cli/issues/14193 PR Close #30085 --- .../writing/new_entry_point_file_writer.ts | 8 ++-- .../new_entry_point_file_writer_spec.ts | 40 +++++++++++++------ 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/writing/new_entry_point_file_writer.ts b/packages/compiler-cli/ngcc/src/writing/new_entry_point_file_writer.ts index c4aea4b16f..1c6edcd099 100644 --- a/packages/compiler-cli/ngcc/src/writing/new_entry_point_file_writer.ts +++ b/packages/compiler-cli/ngcc/src/writing/new_entry_point_file_writer.ts @@ -35,15 +35,17 @@ export class NewEntryPointFileWriter extends InPlaceFileWriter { const relativeEntryPointPath = relative(entryPoint.package, entryPoint.path); const relativeNewDir = join(NGCC_DIRECTORY, relativeEntryPointPath); const newDir = AbsoluteFsPath.fromUnchecked(join(entryPoint.package, relativeNewDir)); - this.copyBundle(bundle, entryPoint.path, newDir); + this.copyBundle(bundle, entryPoint.package, entryPoint.path, newDir); transformedFiles.forEach(file => this.writeFile(file, entryPoint.path, newDir)); this.updatePackageJson(entryPoint, bundle.formatProperty, newDir); } protected copyBundle( - bundle: EntryPointBundle, entryPointPath: AbsoluteFsPath, newDir: AbsoluteFsPath) { + bundle: EntryPointBundle, packagePath: AbsoluteFsPath, entryPointPath: AbsoluteFsPath, + newDir: AbsoluteFsPath) { bundle.src.program.getSourceFiles().forEach(sourceFile => { - if (!sourceFile.isDeclarationFile) { + const isOutsidePackage = relative(packagePath, sourceFile.fileName).startsWith('..'); + if (!sourceFile.isDeclarationFile && !isOutsidePackage) { const relativePath = relative(entryPointPath, sourceFile.fileName); const newFilePath = join(newDir, relativePath); mkdir('-p', dirname(newFilePath)); diff --git a/packages/compiler-cli/ngcc/test/writing/new_entry_point_file_writer_spec.ts b/packages/compiler-cli/ngcc/test/writing/new_entry_point_file_writer_spec.ts index 210379159d..60d44a39b2 100644 --- a/packages/compiler-cli/ngcc/test/writing/new_entry_point_file_writer_spec.ts +++ b/packages/compiler-cli/ngcc/test/writing/new_entry_point_file_writer_spec.ts @@ -30,7 +30,7 @@ function createMockFileSystem() { 'esm5.js': 'export function FooTop() {}', 'esm5.js.map': 'ORIGINAL MAPPING DATA', 'es2015': { - 'index.js': 'import {FooTop} from "./foo";', + 'index.js': 'export {FooTop} from "./foo";', 'foo.js': 'export class FooTop {}', }, 'a': { @@ -40,7 +40,7 @@ function createMockFileSystem() { 'index.metadata.json': '...', 'esm5.js': 'export function FooA() {}', 'es2015': { - 'index.js': 'import {FooA} from "./foo";', + 'index.js': 'export {FooA} from "./foo";', 'foo.js': 'export class FooA {}', }, }, @@ -52,8 +52,8 @@ function createMockFileSystem() { 'lib': { 'esm5.js': 'export function FooB() {}', 'es2015': { - 'index.js': 'import {FooB} from "./foo"; import * from "other";', - 'foo.js': 'export class FooB {}', + 'index.js': 'export {FooB} from "./foo"; import * from "other";', + 'foo.js': 'import {FooA} from "test/a"; import "events"; export class FooB {}', }, }, 'typings': { @@ -66,6 +66,10 @@ function createMockFileSystem() { 'index.d.ts': 'export declare class OtherClass {}', 'esm5.js': 'export class OtherClass {}', }, + '/node_modules/events': { + 'package.json': '{"main": "./events.js"}', + 'events.js': 'export class OtherClass {}', + }, }); } @@ -115,9 +119,9 @@ describe('NewEntryPointFileWriter', () => { expect(readFileSync('/node_modules/test/es2015/foo.js', 'utf8')) .toEqual('export class FooTop {}'); expect(readFileSync('/node_modules/test/__ivy_ngcc__/es2015/index.js', 'utf8')) - .toEqual('import {FooTop} from "./foo";'); + .toEqual('export {FooTop} from "./foo";'); expect(readFileSync('/node_modules/test/es2015/index.js', 'utf8')) - .toEqual('import {FooTop} from "./foo";'); + .toEqual('export {FooTop} from "./foo";'); }); it('should update the package.json properties', () => { @@ -187,9 +191,9 @@ describe('NewEntryPointFileWriter', () => { expect(readFileSync('/node_modules/test/a/es2015/foo.js', 'utf8')) .toEqual('export class FooA {}'); expect(readFileSync('/node_modules/test/__ivy_ngcc__/a/es2015/index.js', 'utf8')) - .toEqual('import {FooA} from "./foo";'); + .toEqual('export {FooA} from "./foo";'); expect(readFileSync('/node_modules/test/a/es2015/index.js', 'utf8')) - .toEqual('import {FooA} from "./foo";'); + .toEqual('export {FooA} from "./foo";'); }); it('should update the package.json properties', () => { @@ -253,14 +257,25 @@ describe('NewEntryPointFileWriter', () => { expect(readFileSync('/node_modules/test/__ivy_ngcc__/lib/es2015/foo.js', 'utf8')) .toEqual('export class FooB {} // MODIFIED'); expect(readFileSync('/node_modules/test/lib/es2015/foo.js', 'utf8')) - .toEqual('export class FooB {}'); + .toEqual('import {FooA} from "test/a"; import "events"; export class FooB {}'); expect(readFileSync('/node_modules/test/__ivy_ngcc__/lib/es2015/index.js', 'utf8')) - .toEqual('import {FooB} from "./foo"; import * from "other";'); + .toEqual('export {FooB} from "./foo"; import * from "other";'); expect(readFileSync('/node_modules/test/lib/es2015/index.js', 'utf8')) - .toEqual('import {FooB} from "./foo"; import * from "other";'); + .toEqual('export {FooB} from "./foo"; import * from "other";'); }); - it('should not copy declaration files outside of the entry-point', () => { + it('should not copy typings files within the package (i.e. from a different entry-point)', + () => { + fileWriter.writeBundle(entryPoint, esm2015bundle, [ + { + path: '/node_modules/test/lib/es2015/foo.js', + contents: 'export class FooB {} // MODIFIED' + }, + ]); + expect(existsSync('/node_modules/test/__ivy_ngcc__/a/index.d.ts')).toEqual(false); + }); + + it('should not copy files outside of the package', () => { fileWriter.writeBundle(entryPoint, esm2015bundle, [ { path: '/node_modules/test/lib/es2015/foo.js', @@ -268,6 +283,7 @@ describe('NewEntryPointFileWriter', () => { }, ]); expect(existsSync('/node_modules/test/other/index.d.ts')).toEqual(false); + expect(existsSync('/node_modules/test/events/events.js')).toEqual(false); }); it('should update the package.json properties', () => {