fix(ngcc): copy (and update) source-maps for unmodified source files (#40429)

When using the `NewEntryPointWriter` we copy unmodified files over to the new
entry-point in addition to writing out the source files that are processed by ngcc.
But we were not copying over associated source-map files for these unmodified
source files, leading to warnings in downstream tooling.

Now we will also copy over source-maps that reside as siblings of unmodified
source files. We have to make sure that the sources of the source-map point
to the correct files, so we also update the `sourceRoot` property of the copied
source-map.

Fixes #40358

PR Close #40429
This commit is contained in:
Pete Bacon Darwin 2021-01-14 11:55:57 +00:00 committed by Andrew Kushnir
parent 9b0b2dd688
commit ad0fb9c6bb
2 changed files with 122 additions and 4 deletions

View File

@ -69,16 +69,49 @@ export class NewEntryPointFileWriter extends InPlaceFileWriter {
protected copyBundle( protected copyBundle(
bundle: EntryPointBundle, packagePath: AbsoluteFsPath, ngccFolder: AbsoluteFsPath) { bundle: EntryPointBundle, packagePath: AbsoluteFsPath, ngccFolder: AbsoluteFsPath) {
bundle.src.program.getSourceFiles().forEach(sourceFile => { bundle.src.program.getSourceFiles().forEach(sourceFile => {
const relativePath = this.fs.relative(packagePath, absoluteFromSourceFile(sourceFile)); const originalPath = absoluteFromSourceFile(sourceFile);
const relativePath = this.fs.relative(packagePath, originalPath);
const isInsidePackage = isLocalRelativePath(relativePath); const isInsidePackage = isLocalRelativePath(relativePath);
if (!sourceFile.isDeclarationFile && isInsidePackage) { if (!sourceFile.isDeclarationFile && isInsidePackage) {
const newFilePath = this.fs.resolve(ngccFolder, relativePath); const newPath = this.fs.resolve(ngccFolder, relativePath);
this.fs.ensureDir(this.fs.dirname(newFilePath)); this.fs.ensureDir(this.fs.dirname(newPath));
this.fs.copyFile(absoluteFromSourceFile(sourceFile), newFilePath); this.fs.copyFile(originalPath, newPath);
this.copyAndUpdateSourceMap(originalPath, newPath);
} }
}); });
} }
/**
* If a source file has an associated source-map, then copy this, while updating its sourceRoot
* accordingly.
*
* For now don't try to parse the source for inline source-maps or external source-map links,
* since that is more complex and will slow ngcc down.
* Instead just check for a source-map file residing next to the source file, which is by far
* the most common case.
*
* @param originalSrcPath absolute path to the original source file being copied.
* @param newSrcPath absolute path to where the source will be written.
*/
protected copyAndUpdateSourceMap(originalSrcPath: AbsoluteFsPath, newSrcPath: AbsoluteFsPath):
void {
const sourceMapPath = (originalSrcPath + '.map') as AbsoluteFsPath;
if (this.fs.exists(sourceMapPath)) {
try {
const sourceMap = JSON.parse(this.fs.readFile(sourceMapPath));
const newSourceMapPath = (newSrcPath + '.map') as AbsoluteFsPath;
const relativePath =
this.fs.relative(this.fs.dirname(newSourceMapPath), this.fs.dirname(sourceMapPath));
sourceMap['sourceRoot'] = this.fs.join(relativePath, sourceMap['sourceRoot'] || '.');
this.fs.ensureDir(this.fs.dirname(newSourceMapPath));
this.fs.writeFile(newSourceMapPath, JSON.stringify(sourceMap));
} catch (e) {
this.logger.warn(`Failed to process source-map at ${sourceMapPath}`);
this.logger.warn(e.message ?? e);
}
}
}
protected writeFile(file: FileToWrite, packagePath: AbsoluteFsPath, ngccFolder: AbsoluteFsPath): protected writeFile(file: FileToWrite, packagePath: AbsoluteFsPath, ngccFolder: AbsoluteFsPath):
void { void {
if (isDtsPath(file.path.replace(/\.map$/, ''))) { if (isDtsPath(file.path.replace(/\.map$/, ''))) {

View File

@ -51,6 +51,12 @@ runInEachFileSystem(() => {
{name: _('/node_modules/test/esm5.js'), contents: 'export function FooTop() {}'}, {name: _('/node_modules/test/esm5.js'), contents: 'export function FooTop() {}'},
{name: _('/node_modules/test/esm5.js.map'), contents: 'ORIGINAL MAPPING DATA'}, {name: _('/node_modules/test/esm5.js.map'), contents: 'ORIGINAL MAPPING DATA'},
{name: _('/node_modules/test/es2015/index.js'), contents: 'export {FooTop} from "./foo";'}, {name: _('/node_modules/test/es2015/index.js'), contents: 'export {FooTop} from "./foo";'},
{
name: _('/node_modules/test/es2015/index.js.map'),
contents:
'{"version":3,"file":"index.js","sources":["../src/index.ts"],"mappings":"AAAA"}'
},
{name: _('/node_modules/test/src/index.ts'), contents: 'export {FooTop} from "./foo";'},
{name: _('/node_modules/test/es2015/foo.js'), contents: 'export class FooTop {}'}, {name: _('/node_modules/test/es2015/foo.js'), contents: 'export class FooTop {}'},
{ {
name: _('/node_modules/test/a/package.json'), name: _('/node_modules/test/a/package.json'),
@ -154,6 +160,85 @@ runInEachFileSystem(() => {
.toEqual('export {FooTop} from "./foo";'); .toEqual('export {FooTop} from "./foo";');
}); });
it('should copy any source-map for unmodified files in the program (adding missing sourceRoot)',
() => {
// Ensure source-mapping for a non-processed source file `index.js`.
const sourceMap = {
version: 3,
file: 'index.js',
sources: ['../src/index.ts'],
mappings: 'AAAA',
};
loadTestFiles([
{
name: _('/node_modules/test/es2015/index.js.map'),
contents: JSON.stringify(sourceMap)
},
{name: _('/node_modules/test/src/index.ts'), contents: 'export {FooTop} from "./foo";'}
]);
// Simulate that only the `foo.js` file was modified
const modifiedFiles = [{
path: _('/node_modules/test/es2015/foo.js'),
contents: 'export class FooTop {} // MODIFIED'
}];
fileWriter.writeBundle(esm2015bundle, modifiedFiles, ['es2015']);
expect(JSON.parse(fs.readFile(_('/node_modules/test/__ivy_ngcc__/es2015/index.js.map'))))
.toEqual({...sourceMap, sourceRoot: '../../es2015'});
});
it('should copy any source-map for unmodified files in the program (updating sourceRoot)',
() => {
// Ensure source-mapping for a non-processed source file `index.js`.
const sourceMap = {
version: 3,
file: 'index.js',
sourceRoot: '../src',
sources: ['index.ts'],
mappings: 'AAAA',
};
loadTestFiles([
{
name: _('/node_modules/test/es2015/index.js.map'),
contents: JSON.stringify(sourceMap)
},
{name: _('/node_modules/test/src/index.ts'), contents: 'export {FooTop} from "./foo";'}
]);
// Simulate that only the `foo.js` file was modified
const modifiedFiles = [{
path: _('/node_modules/test/es2015/foo.js'),
contents: 'export class FooTop {} // MODIFIED'
}];
fileWriter.writeBundle(esm2015bundle, modifiedFiles, ['es2015']);
expect(JSON.parse(fs.readFile(_('/node_modules/test/__ivy_ngcc__/es2015/index.js.map'))))
.toEqual({...sourceMap, sourceRoot: '../../src'});
});
it('should ignore (with a warning) any invalid source-map for unmodified files in the program',
() => {
// Ensure source-mapping for a non-processed source file `index.js`.
loadTestFiles([
{name: _('/node_modules/test/es2015/index.js.map'), contents: 'INVALID JSON STRING'},
{name: _('/node_modules/test/src/index.ts'), contents: 'export {FooTop} from "./foo";'}
]);
// Simulate that only the `foo.js` file was modified
const modifiedFiles = [{
path: _('/node_modules/test/es2015/foo.js'),
contents: 'export class FooTop {} // MODIFIED'
}];
fileWriter.writeBundle(esm2015bundle, modifiedFiles, ['es2015']);
expect(fs.exists(_('/node_modules/test/__ivy_ngcc__/es2015/index.js.map'))).toBe(false);
expect(logger.logs.warn).toEqual([
[`Failed to process source-map at ${_('/node_modules/test/es2015/index.js.map')}`],
['Unexpected token I in JSON at position 0'],
]);
});
it('should update the package.json properties', () => { it('should update the package.json properties', () => {
fileWriter.writeBundle( fileWriter.writeBundle(
esm5bundle, esm5bundle,