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
This commit is contained in:
Pete Bacon Darwin 2019-04-24 14:47:50 +01:00 committed by Andrew Kushnir
parent 0b5f480eca
commit 6af9b8fb92
2 changed files with 33 additions and 15 deletions

View File

@ -35,15 +35,17 @@ export class NewEntryPointFileWriter extends InPlaceFileWriter {
const relativeEntryPointPath = relative(entryPoint.package, entryPoint.path); const relativeEntryPointPath = relative(entryPoint.package, entryPoint.path);
const relativeNewDir = join(NGCC_DIRECTORY, relativeEntryPointPath); const relativeNewDir = join(NGCC_DIRECTORY, relativeEntryPointPath);
const newDir = AbsoluteFsPath.fromUnchecked(join(entryPoint.package, relativeNewDir)); 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)); transformedFiles.forEach(file => this.writeFile(file, entryPoint.path, newDir));
this.updatePackageJson(entryPoint, bundle.formatProperty, newDir); this.updatePackageJson(entryPoint, bundle.formatProperty, newDir);
} }
protected copyBundle( protected copyBundle(
bundle: EntryPointBundle, entryPointPath: AbsoluteFsPath, newDir: AbsoluteFsPath) { bundle: EntryPointBundle, packagePath: AbsoluteFsPath, entryPointPath: AbsoluteFsPath,
newDir: AbsoluteFsPath) {
bundle.src.program.getSourceFiles().forEach(sourceFile => { 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 relativePath = relative(entryPointPath, sourceFile.fileName);
const newFilePath = join(newDir, relativePath); const newFilePath = join(newDir, relativePath);
mkdir('-p', dirname(newFilePath)); mkdir('-p', dirname(newFilePath));

View File

@ -30,7 +30,7 @@ function createMockFileSystem() {
'esm5.js': 'export function FooTop() {}', 'esm5.js': 'export function FooTop() {}',
'esm5.js.map': 'ORIGINAL MAPPING DATA', 'esm5.js.map': 'ORIGINAL MAPPING DATA',
'es2015': { 'es2015': {
'index.js': 'import {FooTop} from "./foo";', 'index.js': 'export {FooTop} from "./foo";',
'foo.js': 'export class FooTop {}', 'foo.js': 'export class FooTop {}',
}, },
'a': { 'a': {
@ -40,7 +40,7 @@ function createMockFileSystem() {
'index.metadata.json': '...', 'index.metadata.json': '...',
'esm5.js': 'export function FooA() {}', 'esm5.js': 'export function FooA() {}',
'es2015': { 'es2015': {
'index.js': 'import {FooA} from "./foo";', 'index.js': 'export {FooA} from "./foo";',
'foo.js': 'export class FooA {}', 'foo.js': 'export class FooA {}',
}, },
}, },
@ -52,8 +52,8 @@ function createMockFileSystem() {
'lib': { 'lib': {
'esm5.js': 'export function FooB() {}', 'esm5.js': 'export function FooB() {}',
'es2015': { 'es2015': {
'index.js': 'import {FooB} from "./foo"; import * from "other";', 'index.js': 'export {FooB} from "./foo"; import * from "other";',
'foo.js': 'export class FooB {}', 'foo.js': 'import {FooA} from "test/a"; import "events"; export class FooB {}',
}, },
}, },
'typings': { 'typings': {
@ -66,6 +66,10 @@ function createMockFileSystem() {
'index.d.ts': 'export declare class OtherClass {}', 'index.d.ts': 'export declare class OtherClass {}',
'esm5.js': 'export 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')) expect(readFileSync('/node_modules/test/es2015/foo.js', 'utf8'))
.toEqual('export class FooTop {}'); .toEqual('export class FooTop {}');
expect(readFileSync('/node_modules/test/__ivy_ngcc__/es2015/index.js', 'utf8')) 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')) 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', () => { it('should update the package.json properties', () => {
@ -187,9 +191,9 @@ describe('NewEntryPointFileWriter', () => {
expect(readFileSync('/node_modules/test/a/es2015/foo.js', 'utf8')) expect(readFileSync('/node_modules/test/a/es2015/foo.js', 'utf8'))
.toEqual('export class FooA {}'); .toEqual('export class FooA {}');
expect(readFileSync('/node_modules/test/__ivy_ngcc__/a/es2015/index.js', 'utf8')) 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')) 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', () => { 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')) expect(readFileSync('/node_modules/test/__ivy_ngcc__/lib/es2015/foo.js', 'utf8'))
.toEqual('export class FooB {} // MODIFIED'); .toEqual('export class FooB {} // MODIFIED');
expect(readFileSync('/node_modules/test/lib/es2015/foo.js', 'utf8')) 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')) 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')) 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, [ fileWriter.writeBundle(entryPoint, esm2015bundle, [
{ {
path: '/node_modules/test/lib/es2015/foo.js', 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/other/index.d.ts')).toEqual(false);
expect(existsSync('/node_modules/test/events/events.js')).toEqual(false);
}); });
it('should update the package.json properties', () => { it('should update the package.json properties', () => {