Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ngcc): handle missing sources and out of range mappings when flattening source-maps #35718

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/compiler-cli/ngcc/src/rendering/dts_renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export class DtsRenderer {
this.dtsFormatter.addImports(
outputText, importManager.getAllImports(dtsFile.fileName), dtsFile);

return renderSourceAndMap(this.fs, dtsFile, outputText);
return renderSourceAndMap(this.logger, this.fs, dtsFile, outputText);
}

private getTypingsFilesToRender(
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/ngcc/src/rendering/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export class Renderer {
}

if (compiledFile || switchMarkerAnalysis || isEntryPoint) {
return renderSourceAndMap(this.fs, sourceFile, outputText);
return renderSourceAndMap(this.logger, this.fs, sourceFile, outputText);
} else {
return [];
}
Expand Down
40 changes: 26 additions & 14 deletions packages/compiler-cli/ngcc/src/rendering/source_maps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {FileSystem, absoluteFromSourceFile, basename, absoluteFrom} from '../../
import {FileToWrite} from './utils';
import {SourceFileLoader} from '../sourcemaps/source_file_loader';
import {RawSourceMap} from '../sourcemaps/raw_source_map';
import {Logger} from '../logging/logger';

export interface SourceMapInfo {
source: string;
Expand All @@ -24,28 +25,39 @@ export interface SourceMapInfo {
* with an appropriate source-map comment pointing to the merged source-map.
*/
export function renderSourceAndMap(
fs: FileSystem, sourceFile: ts.SourceFile, generatedMagicString: MagicString): FileToWrite[] {
logger: Logger, fs: FileSystem, sourceFile: ts.SourceFile,
generatedMagicString: MagicString): FileToWrite[] {
const generatedPath = absoluteFromSourceFile(sourceFile);
const generatedMapPath = absoluteFrom(`${generatedPath}.map`);
const generatedContent = generatedMagicString.toString();
const generatedMap: RawSourceMap = generatedMagicString.generateMap(
{file: generatedPath, source: generatedPath, includeContent: true});

const loader = new SourceFileLoader(fs);
const generatedFile = loader.loadSourceFile(
generatedPath, generatedContent, {map: generatedMap, mapPath: generatedMapPath});
try {
const loader = new SourceFileLoader(fs);
const generatedFile = loader.loadSourceFile(
generatedPath, generatedContent, {map: generatedMap, mapPath: generatedMapPath});

const rawMergedMap: RawSourceMap = generatedFile.renderFlattenedSourceMap();
const mergedMap = fromObject(rawMergedMap);

if (generatedFile.sources[0]?.inline) {
// The input source-map was inline so make the output one inline too.
return [{path: generatedPath, contents: `${generatedFile.contents}\n${mergedMap.toComment()}`}];
} else {
const sourceMapComment = generateMapFileComment(`${basename(generatedPath)}.map`);
const rawMergedMap: RawSourceMap = generatedFile.renderFlattenedSourceMap();
const mergedMap = fromObject(rawMergedMap);
if (generatedFile.sources[0]?.inline) {
// The input source-map was inline so make the output one inline too.
return [
{path: generatedPath, contents: `${generatedFile.contents}\n${mergedMap.toComment()}`}
];
} else {
const sourceMapComment = generateMapFileComment(`${basename(generatedPath)}.map`);
return [
{path: generatedPath, contents: `${generatedFile.contents}\n${sourceMapComment}`},
{path: generatedMapPath, contents: mergedMap.toJSON()}
];
}
} catch (e) {
logger.error(
`Error when flattening the source-map "${generatedMapPath}" for "${generatedPath}": ${e.toString()}`);
return [
{path: generatedPath, contents: `${generatedFile.contents}\n${sourceMapComment}`},
{path: generatedMapPath, contents: mergedMap.toJSON()}
{path: generatedPath, contents: generatedContent},
{path: generatedMapPath, contents: fromObject(generatedMap).toJSON()},
];
}
}
20 changes: 15 additions & 5 deletions packages/compiler-cli/ngcc/src/sourcemaps/source_file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,9 @@ export class SourceFile {
const sources: SourceFile[] = [];
const names: string[] = [];

// Ensure a mapping line array for each line in the generated source.
const mappings: SourceMapMappings = this.lineLengths.map(() => []);
const mappings: SourceMapMappings = [];

for (const mapping of this.flattenedMappings) {
const mappingLine = mappings[mapping.generatedSegment.line];
const sourceIndex = findIndexOrAdd(sources, mapping.originalSource);
const mappingArray: SourceMapSegment = [
mapping.generatedSegment.column,
Expand All @@ -65,7 +63,14 @@ export class SourceFile {
const nameIndex = findIndexOrAdd(names, mapping.name);
mappingArray.push(nameIndex);
}
mappingLine.push(mappingArray);

// Ensure a mapping line array for this mapping.
const line = mapping.generatedSegment.line;
while (line >= mappings.length) {
mappings.push([]);
}
// Add this mapping to the line
mappings[line].push(mappingArray);
}

const sourcePathDir = dirname(this.sourcePath);
Expand Down Expand Up @@ -290,11 +295,16 @@ export function parseMappings(
const generatedLineMappings = rawMappings[generatedLine];
for (const rawMapping of generatedLineMappings) {
if (rawMapping.length >= 4) {
const originalSource = sources[rawMapping[1] !];
if (originalSource === null || originalSource === undefined) {
// the original source is missing so ignore this mapping
continue;
}
const generatedColumn = rawMapping[0];
const name = rawMapping.length === 5 ? rawMap.names[rawMapping[4]] : undefined;
const mapping: Mapping = {
generatedSegment: {line: generatedLine, column: generatedColumn},
originalSource: sources[rawMapping[1] !] !,
originalSource,
originalSegment: {line: rawMapping[2] !, column: rawMapping[3] !}, name
};
mappings.push(mapping);
Expand Down
8 changes: 2 additions & 6 deletions packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ runInEachFileSystem(() => {
[5, 0, 1, 20], [7, 0, 1, 22], [12, 0, 1, 27], [14, 0, 1, 28], [15, 0, 1, 29],
[9, 0, 2, 13], [10, 0, 2, 14]
],
[],
];

JS_CONTENT_MAP = fromObject({
Expand All @@ -196,15 +195,12 @@ runInEachFileSystem(() => {
'file': 'file.js',
'sources': ['file.js'],
'names': [],
'mappings': encode([
[], [], [], [], [], [], [], [], [], [], [], [], [[0, 0, 0, 0]],
[], [], [], [], [], [], [], [], []
]),
'mappings': encode([[], [], [], [], [], [], [], [], [], [], [], [], [[0, 0, 0, 0]]]),
'sourcesContent': [JS_CONTENT.contents],
});

const MERGED_OUTPUT_PROGRAM_MAPPINGS: SourceMapMappings =
[[], [], [], [], [], [], [], [], [], [], [], [], ...JS_CONTENT_MAPPINGS, []];
[[], [], [], [], [], [], [], [], [], [], [], [], ...JS_CONTENT_MAPPINGS];

MERGED_OUTPUT_PROGRAM_MAP = fromObject({
'version': 3,
Expand Down
48 changes: 48 additions & 0 deletions packages/compiler-cli/ngcc/test/sourcemaps/source_file_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,28 @@ runInEachFileSystem(() => {
},
]);
});

it('should ignore mappings to missing source files', () => {
const bSourceMap: RawSourceMap = {
mappings: encode([[[1, 0, 0, 0], [4, 0, 0, 3], [4, 0, 0, 6], [5, 0, 0, 7]]]),
names: [],
sources: ['c.js'],
version: 3
};
const bSource = new SourceFile(_('/foo/src/b.js'), 'abcdef', bSourceMap, false, [null]);
const aSourceMap: RawSourceMap = {
mappings: encode([[[0, 0, 0, 0], [2, 0, 0, 3], [4, 0, 0, 2], [5, 0, 0, 5]]]),
names: [],
sources: ['b.js'],
version: 3
};
const aSource =
new SourceFile(_('/foo/src/a.js'), 'abdecf', aSourceMap, false, [bSource]);

// These flattened mappings are just the mappings from a to b.
// (The mappings to c are dropped since there is no source file to map to.)
expect(aSource.flattenedMappings).toEqual(parseMappings(aSourceMap, [bSource]));
});
});

describe('renderFlattenedSourceMap()', () => {
Expand Down Expand Up @@ -207,6 +229,32 @@ runInEachFileSystem(() => {
[[1, 0, 0, 0], [2, 0, 0, 2], [3, 0, 0, 3], [3, 0, 0, 6], [4, 0, 0, 1], [5, 0, 0, 7]]
]));
});

it('should handle mappings that map from lines outside of the actual content lines', () => {
const bSource = new SourceFile(_('/foo/src/b.js'), 'abcdef', null, false, []);
const aToBSourceMap: RawSourceMap = {
mappings: encode([
[[0, 0, 0, 0], [2, 0, 0, 3], [4, 0, 0, 2], [5, 0, 0, 5]],
[
[0, 0, 0, 0], // Extra mapping from a non-existent line
]
]),
names: [],
sources: ['b.js'],
version: 3
};
const aSource =
new SourceFile(_('/foo/src/a.js'), 'abdecf', aToBSourceMap, false, [bSource]);

const aTocSourceMap = aSource.renderFlattenedSourceMap();
expect(aTocSourceMap.version).toEqual(3);
expect(aTocSourceMap.file).toEqual('a.js');
expect(aTocSourceMap.names).toEqual([]);
expect(aTocSourceMap.sourceRoot).toBeUndefined();
expect(aTocSourceMap.sources).toEqual(['b.js']);
expect(aTocSourceMap.sourcesContent).toEqual(['abcdef']);
expect(aTocSourceMap.mappings).toEqual(aToBSourceMap.mappings);
});
});
});

Expand Down