From b45f336635adde248b49ddf9dcd6b3a7d47409ca Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Sat, 30 May 2020 20:49:44 +0100 Subject: [PATCH] fix(ngcc): do not inline source-maps for non-inline typings source-maps (#37363) Inline source-maps in typings files can impact IDE performance so ngcc should only add such maps if the original typings file contains inline source-maps. Fixes #37324 PR Close #37363 --- .../ngcc/src/rendering/source_maps.ts | 12 ++++- .../ngcc/test/rendering/dts_renderer_spec.ts | 44 +++++++++++++++++++ .../ngcc/test/rendering/renderer_spec.ts | 18 ++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/rendering/source_maps.ts b/packages/compiler-cli/ngcc/src/rendering/source_maps.ts index 23be47407b..4e29fd090d 100644 --- a/packages/compiler-cli/ngcc/src/rendering/source_maps.ts +++ b/packages/compiler-cli/ngcc/src/rendering/source_maps.ts @@ -42,8 +42,16 @@ export function renderSourceAndMap( 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. + const firstSource = generatedFile.sources[0]; + if (firstSource && (firstSource.rawMap !== null || !sourceFile.isDeclarationFile) && + firstSource.inline) { + // We render an inline source map if one of: + // * there was no input source map and this is not a typings file; + // * the input source map exists and was inline. + // + // We do not generate inline source maps for typings files unless there explicitly was one in + // the input file because these inline source maps can be very large and it impacts on the + // performance of IDEs that need to read them to provide intellisense etc. return [ {path: generatedPath, contents: `${generatedFile.contents}\n${mergedMap.toComment()}`} ]; diff --git a/packages/compiler-cli/ngcc/test/rendering/dts_renderer_spec.ts b/packages/compiler-cli/ngcc/test/rendering/dts_renderer_spec.ts index b8a3cd2ad3..657340c37a 100644 --- a/packages/compiler-cli/ngcc/test/rendering/dts_renderer_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/dts_renderer_spec.ts @@ -5,7 +5,9 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {fromObject} from 'convert-source-map'; import MagicString from 'magic-string'; +import {encode} from 'sourcemap-codec'; import * as ts from 'typescript'; import {absoluteFrom, getFileSystem} from '../../../src/ngtsc/file_system'; @@ -195,5 +197,47 @@ runInEachFileSystem(() => { result.find(f => f.path === _('/node_modules/test-package/typings/file.d.ts'))!; expect(typingsFile.contents).toContain(`\n// ADD MODUlE WITH PROVIDERS PARAMS\n`); }); + + it('should render an external source map for files whose original file does not have a source map', + () => { + const { + renderer, + decorationAnalyses, + privateDeclarationsAnalyses, + moduleWithProvidersAnalyses + } = createTestRenderer('test-package', [INPUT_PROGRAM], [INPUT_DTS_PROGRAM]); + + const result = renderer.renderProgram( + decorationAnalyses, privateDeclarationsAnalyses, moduleWithProvidersAnalyses); + + const typingsFile = + result.find(f => f.path === _('/node_modules/test-package/typings/file.d.ts'))!; + expect(typingsFile.contents).toContain('//# sourceMappingURL=file.d.ts.map'); + }); + + it('should render an internal source map for files whose original file has an internal source map', + () => { + const sourceMap = fromObject({ + 'version': 3, + 'file': 'file.d.ts', + 'sources': ['file.d.ts'], + 'names': [], + 'mappings': encode([[]]), + 'sourcesContent': [INPUT_DTS_PROGRAM.contents], + }); + INPUT_DTS_PROGRAM.contents += sourceMap.toComment(); + const { + renderer, + decorationAnalyses, + privateDeclarationsAnalyses, + moduleWithProvidersAnalyses + } = createTestRenderer('test-package', [INPUT_PROGRAM], [INPUT_DTS_PROGRAM]); + const result = renderer.renderProgram( + decorationAnalyses, privateDeclarationsAnalyses, moduleWithProvidersAnalyses); + + const typingsFile = + result.find(f => f.path === _('/node_modules/test-package/typings/file.d.ts'))!; + expect(typingsFile.contents).toContain('//# sourceMappingURL=data:application/json'); + }); }); }); diff --git a/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts b/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts index bc7e44a879..e8011f0648 100644 --- a/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts @@ -640,6 +640,24 @@ UndecoratedBase.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: UndecoratedBase, vie expect(mapFile.path).toEqual(_('/node_modules/test-package/src/file.js.map')); expect(JSON.parse(mapFile.contents)).toEqual(MERGED_OUTPUT_PROGRAM_MAP.toObject()); }); + + + it('should render an internal source map for files whose original file does not have a source map', + () => { + const sourceFiles: TestFile[] = [JS_CONTENT]; + const { + decorationAnalyses, + renderer, + switchMarkerAnalyses, + privateDeclarationsAnalyses + } = createTestRenderer('test-package', sourceFiles, undefined); + const [sourceFile, mapFile] = renderer.renderProgram( + decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); + expect(sourceFile.path).toEqual(_('/node_modules/test-package/src/file.js')); + expect(sourceFile.contents) + .toEqual(RENDERED_CONTENTS + '\n' + OUTPUT_PROGRAM_MAP.toComment()); + expect(mapFile).toBeUndefined(); + }); }); describe('@angular/core support', () => {