From f08d79b1d14e4483332f59877056c32399efb2e2 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Thu, 2 Jan 2020 17:41:24 +0100 Subject: [PATCH] fix(@angular-devkit/build-angular): add sourceMappingURL comment for ES2015 during differential loading When having differential loading enabled we only add the `sourceMappingURL` comment when optimization is enabled, because we only process these bundles when we enabling optimization. With this change we now process such bundles even when optimization is disabled and add `sourceMappingURL` when source maps are enabled and not hidden. Closes #16522 --- .../build_angular/src/browser/index.ts | 8 +- .../build_angular/src/utils/action-cache.ts | 27 ++- .../build_angular/src/utils/process-bundle.ts | 203 +++++++----------- .../e2e/tests/build/no-sourcemap.ts | 36 ++++ tests/legacy-cli/e2e/tests/build/sourcemap.ts | 13 +- 5 files changed, 139 insertions(+), 148 deletions(-) create mode 100644 tests/legacy-cli/e2e/tests/build/no-sourcemap.ts diff --git a/packages/angular_devkit/build_angular/src/browser/index.ts b/packages/angular_devkit/build_angular/src/browser/index.ts index fe3f304ac003..029c8cc496b7 100644 --- a/packages/angular_devkit/build_angular/src/browser/index.ts +++ b/packages/angular_devkit/build_angular/src/browser/index.ts @@ -390,12 +390,6 @@ export function buildWebpackBrowser( if (!es5Polyfills) { moduleFiles.push(file); } - // If not optimizing then ES2015 polyfills do not need processing - // Unlike other module scripts, it is never downleveled - const es2015Polyfills = file.file.startsWith('polyfills-es20'); - if (!actionOptions.optimize && es2015Polyfills) { - continue; - } // Retrieve the content/map for the file // NOTE: Additional future optimizations will read directly from memory @@ -417,6 +411,8 @@ export function buildWebpackBrowser( filename = filename.replace(/\-es20\d{2}/, ''); } + const es2015Polyfills = file.file.startsWith('polyfills-es20'); + // Record the bundle processing action // The runtime chunk gets special processing for lazy loaded files actions.push({ diff --git a/packages/angular_devkit/build_angular/src/utils/action-cache.ts b/packages/angular_devkit/build_angular/src/utils/action-cache.ts index 97c37f9d568f..f0f54b352995 100644 --- a/packages/angular_devkit/build_angular/src/utils/action-cache.ts +++ b/packages/angular_devkit/build_angular/src/utils/action-cache.ts @@ -50,38 +50,43 @@ export class BundleActionCache { } generateCacheKeys(action: ProcessBundleOptions): string[] { - const baseCacheKey = this.generateBaseCacheKey(action.code); - - // Postfix added to sourcemap cache keys when vendor sourcemaps are present + // Postfix added to sourcemap cache keys when vendor, hidden sourcemaps are present // Allows non-destructive caching of both variants - const SourceMapVendorPostfix = !!action.sourceMaps && action.vendorSourceMaps ? '|vendor' : ''; + const sourceMapVendorPostfix = action.sourceMaps && action.vendorSourceMaps ? '|vendor' : ''; + + // sourceMappingURL is added at the very end which causes the code to be the same when sourcemaps are enabled/disabled + // When using hiddenSourceMaps we can omit the postfix since sourceMappingURL will not be added. + const sourceMapPostFix = action.sourceMaps && !action.hiddenSourceMaps ? '|sourcemap' : ''; + + const baseCacheKey = this.generateBaseCacheKey(action.code); // Determine cache entries required based on build settings - const cacheKeys = []; + const cacheKeys: string[] = []; // If optimizing and the original is not ignored, add original as required - if ((action.optimize || action.optimizeOnly) && !action.ignoreOriginal) { - cacheKeys[CacheKey.OriginalCode] = baseCacheKey + '|orig'; + if (!action.ignoreOriginal) { + cacheKeys[CacheKey.OriginalCode] = baseCacheKey + sourceMapPostFix + '|orig'; // If sourcemaps are enabled, add original sourcemap as required if (action.sourceMaps) { - cacheKeys[CacheKey.OriginalMap] = baseCacheKey + SourceMapVendorPostfix + '|orig-map'; + cacheKeys[CacheKey.OriginalMap] = baseCacheKey + sourceMapVendorPostfix + '|orig-map'; } } + // If not only optimizing, add downlevel as required if (!action.optimizeOnly) { - cacheKeys[CacheKey.DownlevelCode] = baseCacheKey + '|dl'; + cacheKeys[CacheKey.DownlevelCode] = baseCacheKey + sourceMapPostFix + '|dl'; // If sourcemaps are enabled, add downlevel sourcemap as required if (action.sourceMaps) { - cacheKeys[CacheKey.DownlevelMap] = baseCacheKey + SourceMapVendorPostfix + '|dl-map'; + cacheKeys[CacheKey.DownlevelMap] = baseCacheKey + sourceMapVendorPostfix + '|dl-map'; } } return cacheKeys; } - async getCacheEntries(cacheKeys: (string | null)[]): Promise<(CacheEntry | null)[] | false> { + async getCacheEntries(cacheKeys: (string | undefined)[]): Promise<(CacheEntry | null)[] | false> { // Attempt to get required cache entries const cacheEntries = []; for (const key of cacheKeys) { diff --git a/packages/angular_devkit/build_angular/src/utils/process-bundle.ts b/packages/angular_devkit/build_angular/src/utils/process-bundle.ts index f1f8b495b170..bcf8084b73b0 100644 --- a/packages/angular_devkit/build_angular/src/utils/process-bundle.ts +++ b/packages/angular_devkit/build_angular/src/utils/process-bundle.ts @@ -39,7 +39,7 @@ export interface ProcessBundleOptions { optimize?: boolean; optimizeOnly?: boolean; ignoreOriginal?: boolean; - cacheKeys?: (string | null)[]; + cacheKeys?: (string | undefined)[]; integrityAlgorithm?: 'sha256' | 'sha384' | 'sha512'; runtimeData?: ProcessBundleResult[]; replacements?: [string, string][]; @@ -80,9 +80,9 @@ export function setup(data: number[] | { cachePath: string; i18n: I18nOptions }) i18n = options.i18n; } -async function cachePut(content: string, key: string | null, integrity?: string): Promise { +async function cachePut(content: string, key: string | undefined, integrity?: string): Promise { if (cachePath && key) { - await cacache.put(cachePath, key, content, { + await cacache.put(cachePath, key || null, content, { metadata: { integrity }, }); } @@ -114,7 +114,6 @@ export async function process(options: ProcessBundleOptions): Promise= 500 * 1024 || mapSize >= 500 * 1024; - const sourceCode = options.code; const sourceMap = options.map ? JSON.parse(options.map) : undefined; @@ -155,59 +154,21 @@ export async function process(options: ProcessBundleOptions): Promise { - const result = terserMangle(options.code, { - filename: path.basename(options.filename), - map: options.map ? JSON.parse(options.map) : undefined, - ecma: 6, - }); +async function processBundle( + options: Omit & { isOriginal: boolean; map?: string | RawSourceMap }, +): Promise { + const { + optimize, + isOriginal, + code, + map, + filename: filepath, + hiddenSourceMaps, + cacheKeys = [], + integrityAlgorithm, + } = options; + + const rawMap = typeof map === 'string' ? JSON.parse(map) as RawSourceMap : map; + const filename = path.basename(filepath); + + let result: { + code: string, + map: RawSourceMap | undefined, + }; + + if (optimize) { + result = terserMangle(code, { + filename, + map: rawMap, + compress: !isOriginal, // We only compress bundles which are downlevelled. + ecma: isOriginal ? 6 : 5, + }); + } else { + if (rawMap) { + rawMap.file = filename; + } - let mapContent; + result = { + map: rawMap, + code, + }; + } + + let mapContent: string | undefined; if (result.map) { - if (!options.hiddenSourceMaps) { - result.code += `\n//# sourceMappingURL=${path.basename(options.filename)}.map`; + if (!hiddenSourceMaps) { + result.code += `\n//# sourceMappingURL=${filename}.map`; } mapContent = JSON.stringify(result.map); await cachePut( mapContent, - (options.cacheKeys && options.cacheKeys[CacheKey.OriginalMap]) || null, + cacheKeys[isOriginal ? CacheKey.OriginalMap : CacheKey.DownlevelMap], ); - fs.writeFileSync(options.filename + '.map', mapContent); + fs.writeFileSync(filepath + '.map', mapContent); } const fileResult = createFileEntry( - options.filename, + filepath, result.code, mapContent, - options.integrityAlgorithm, + integrityAlgorithm, ); await cachePut( result.code, - (options.cacheKeys && options.cacheKeys[CacheKey.OriginalCode]) || null, + cacheKeys[isOriginal ? CacheKey.OriginalCode : CacheKey.DownlevelCode], fileResult.integrity, ); - fs.writeFileSync(options.filename, result.code); + fs.writeFileSync(filepath, result.code); return fileResult; } @@ -421,66 +415,19 @@ async function processRuntime( // Extra spacing is intentional to align source line positions downlevelCode = downlevelCode.replace(/"\-es20\d{2}\./, ' "-es5.'); - const downlevelFilePath = options.filename.replace(/\-es20\d{2}/, '-es5'); - let downlevelMap; - let result; - if (options.optimize) { - const minifiyResults = terserMangle(downlevelCode, { - filename: path.basename(downlevelFilePath), - map: options.map === undefined ? undefined : JSON.parse(options.map), - }); - downlevelCode = minifiyResults.code; - downlevelMap = JSON.stringify(minifiyResults.map); - - result = { - original: await mangleOriginal({ ...options, code: originalCode }), - downlevel: createFileEntry( - downlevelFilePath, - downlevelCode, - downlevelMap, - options.integrityAlgorithm, - ), - }; - } else { - if (options.map) { - const rawMap = JSON.parse(options.map) as RawSourceMap; - rawMap.file = path.basename(downlevelFilePath); - downlevelMap = JSON.stringify(rawMap); - } - - result = { - original: createFileEntry( - options.filename, - originalCode, - options.map, - options.integrityAlgorithm, - ), - downlevel: createFileEntry( - downlevelFilePath, - downlevelCode, - downlevelMap, - options.integrityAlgorithm, - ), - }; - } - - if (downlevelMap) { - await cachePut( - downlevelMap, - (options.cacheKeys && options.cacheKeys[CacheKey.DownlevelMap]) || null, - ); - fs.writeFileSync(downlevelFilePath + '.map', downlevelMap); - if (!options.hiddenSourceMaps) { - downlevelCode += `\n//# sourceMappingURL=${path.basename(downlevelFilePath)}.map`; - } - } - await cachePut( - downlevelCode, - (options.cacheKeys && options.cacheKeys[CacheKey.DownlevelCode]) || null, - ); - fs.writeFileSync(downlevelFilePath, downlevelCode); - - return result; + return { + original: await processBundle({ + ...options, + code: originalCode, + isOriginal: true, + }), + downlevel: await processBundle({ + ...options, + code: downlevelCode, + filename: options.filename.replace(/\-es20\d{2}/, '-es5'), + isOriginal: false, + }), + }; } function createReplacePlugin(replacements: [string, string][]): PluginObj { diff --git a/tests/legacy-cli/e2e/tests/build/no-sourcemap.ts b/tests/legacy-cli/e2e/tests/build/no-sourcemap.ts new file mode 100644 index 000000000000..bf0468fbfaa3 --- /dev/null +++ b/tests/legacy-cli/e2e/tests/build/no-sourcemap.ts @@ -0,0 +1,36 @@ +import * as fs from 'fs'; +import { ng } from '../../utils/process'; + +export default async function () { + await ng('build', '--prod', '--output-hashing=none', '--source-map', 'false'); + await testForSourceMaps(6); + + await ng('build', '--output-hashing=none', '--source-map', 'false'); + await testForSourceMaps(8); +} + +async function testForSourceMaps(expectedNumberOfFiles: number): Promise { + const files = fs.readdirSync('./dist/test-project'); + + let count = 0; + for (const file of files) { + if (!file.endsWith('.js')) { + continue; + } + + ++count; + + if (files.includes(file + '.map')) { + throw new Error('Sourcemap generated for ' + file); + } + + const content = fs.readFileSync('./dist/test-project/' + file, 'utf8'); + if (content.includes(`//# sourceMappingURL=${file}.map`)) { + throw new Error('Sourcemap comment found generated for ' + file); + } + } + + if (count < expectedNumberOfFiles) { + throw new Error(`Javascript file count is low. Expected ${expectedNumberOfFiles} but found ${count}`); + } +} diff --git a/tests/legacy-cli/e2e/tests/build/sourcemap.ts b/tests/legacy-cli/e2e/tests/build/sourcemap.ts index 21348d7f1d60..94152f9083b6 100644 --- a/tests/legacy-cli/e2e/tests/build/sourcemap.ts +++ b/tests/legacy-cli/e2e/tests/build/sourcemap.ts @@ -2,10 +2,17 @@ import * as fs from 'fs'; import { expectFileToExist } from '../../utils/fs'; import { ng } from '../../utils/process'; -export default async function() { +export default async function () { await ng('build', '--prod', '--output-hashing=none', '--source-map'); + await testForSourceMaps(6); + await ng('build', '--output-hashing=none', '--source-map'); + await testForSourceMaps(8); +} + +async function testForSourceMaps(expectedNumberOfFiles: number): Promise { await expectFileToExist('dist/test-project/main-es5.js.map'); + await expectFileToExist('dist/test-project/main-es2015.js.map'); const files = fs.readdirSync('./dist/test-project'); @@ -29,7 +36,7 @@ export default async function() { } } - if (count < 6) { - throw new Error('Javascript file count is low'); + if (count < expectedNumberOfFiles) { + throw new Error(`Javascript file count is low. Expected ${expectedNumberOfFiles} but found ${count}`); } }