From ba3e0fe54f59e71150aa73af6b64b408f164e22a Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Thu, 21 Oct 2021 09:25:59 -0400 Subject: [PATCH 1/2] fix(@angular-devkit/build-angular): avoid extra filesystem access with i18n and differential loading When both differential loading and i18n inlining were enabled, the differential loading output was written to disk and then immediately read back in by the i18n inlining processing. Since the i18n inlining would then overwrite the differential loading output, the write/read cycle becomes unneeded extra filesystem accesses. Now when both are enabled, the output of differential loading is kept in memory and used directly by the i18n inlining. --- .../build_angular/src/browser/index.ts | 36 ++++++++----------- .../build_angular/src/utils/process-bundle.ts | 23 ++++++++++-- 2 files changed, 35 insertions(+), 24 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/browser/index.ts b/packages/angular_devkit/build_angular/src/browser/index.ts index f3cf6bf01068..a6ce074a2014 100644 --- a/packages/angular_devkit/build_angular/src/browser/index.ts +++ b/packages/angular_devkit/build_angular/src/browser/index.ts @@ -365,7 +365,7 @@ export function buildWebpackBrowser( if (actionOptions.sourceMaps) { try { map = fs.readFileSync(filename + '.map', 'utf8'); - if (es5Polyfills) { + if (es5Polyfills || i18n.shouldInline) { fs.unlinkSync(filename + '.map'); } } catch {} @@ -374,6 +374,10 @@ export function buildWebpackBrowser( if (es5Polyfills) { fs.unlinkSync(filename); filename = filename.replace(/\-es20\d{2}/, ''); + } else if (i18n.shouldInline) { + // Original files must be deleted with i18n to avoid the original files from + // being copied over the translated files when copying the project assets. + fs.unlinkSync(filename); } const es2015Polyfills = file.file.startsWith('polyfills-es20'); @@ -391,6 +395,8 @@ export function buildWebpackBrowser( runtime: file.file.startsWith('runtime'), ignoreOriginal: es5Polyfills, optimizeOnly: es2015Polyfills, + // When using i18n, file results are kept in memory for further processing + memoryMode: i18n.shouldInline, }); // ES2015 polyfills are only optimized; optimization check was performed above @@ -451,41 +457,32 @@ export function buildWebpackBrowser( if (i18n.shouldInline) { spinner.start('Generating localized bundles...'); const inlineActions: InlineOptions[] = []; - const processedFiles = new Set(); for (const result of processResults) { if (result.original) { inlineActions.push({ filename: path.basename(result.original.filename), - code: fs.readFileSync(result.original.filename, 'utf8'), - map: - result.original.map && - fs.readFileSync(result.original.map.filename, 'utf8'), + // Memory mode is always enabled for i18n + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + code: result.original.content!, + map: result.original.map?.content, outputPath: baseOutputPath, es5: false, missingTranslation: options.i18nMissingTranslation, setLocale: result.name === mainChunkId, }); - processedFiles.add(result.original.filename); - if (result.original.map) { - processedFiles.add(result.original.map.filename); - } } if (result.downlevel) { inlineActions.push({ filename: path.basename(result.downlevel.filename), - code: fs.readFileSync(result.downlevel.filename, 'utf8'), - map: - result.downlevel.map && - fs.readFileSync(result.downlevel.map.filename, 'utf8'), + // Memory mode is always enabled for i18n + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + code: result.downlevel.content!, + map: result.downlevel.map?.content, outputPath: baseOutputPath, es5: true, missingTranslation: options.i18nMissingTranslation, setLocale: result.name === mainChunkId, }); - processedFiles.add(result.downlevel.filename); - if (result.downlevel.map) { - processedFiles.add(result.downlevel.map.filename); - } } } @@ -520,9 +517,6 @@ export function buildWebpackBrowser( glob: '**/*', input: webpackOutputPath, output: '', - ignore: [...processedFiles].map((f) => - path.relative(webpackOutputPath, f), - ), }, ], Array.from(outputPaths.values()), 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 04379022fce8..4c511025376b 100644 --- a/packages/angular_devkit/build_angular/src/utils/process-bundle.ts +++ b/packages/angular_devkit/build_angular/src/utils/process-bundle.ts @@ -56,6 +56,7 @@ export interface ProcessBundleOptions { runtimeData?: ProcessBundleResult[]; replacements?: [string, string][]; supportedBrowsers?: string[] | Record; + memoryMode?: boolean; } export interface ProcessBundleResult { @@ -69,9 +70,11 @@ export interface ProcessBundleFile { filename: string; size: number; integrity?: string; + content?: string; map?: { filename: string; size: number; + content?: string; }; } @@ -200,6 +203,7 @@ async function processBundle( hiddenSourceMaps, cacheKeys = [], integrityAlgorithm, + memoryMode, } = options; const filename = path.basename(filepath); @@ -242,17 +246,27 @@ async function processBundle( mapContent, cacheKeys[isOriginal ? CacheKey.OriginalMap : CacheKey.DownlevelMap], ); - fs.writeFileSync(filepath + '.map', mapContent); + if (!memoryMode) { + fs.writeFileSync(filepath + '.map', mapContent); + } } - const fileResult = createFileEntry(filepath, resultCode, mapContent, integrityAlgorithm); + const fileResult = createFileEntry( + filepath, + resultCode, + mapContent, + memoryMode, + integrityAlgorithm, + ); await cachePut( resultCode, cacheKeys[isOriginal ? CacheKey.OriginalCode : CacheKey.DownlevelCode], fileResult.integrity, ); - fs.writeFileSync(filepath, resultCode); + if (!memoryMode) { + fs.writeFileSync(filepath, resultCode); + } return fileResult; } @@ -293,17 +307,20 @@ function createFileEntry( filename: string, code: string, map: string | undefined, + memoryMode?: boolean, integrityAlgorithm?: string, ): ProcessBundleFile { return { filename: filename, size: Buffer.byteLength(code), integrity: integrityAlgorithm && generateIntegrityValue(integrityAlgorithm, code), + content: memoryMode ? code : undefined, map: !map ? undefined : { filename: filename + '.map', size: Buffer.byteLength(map), + content: memoryMode ? map : undefined, }, }; } From 4517b1632a62c30faba1e9bcd5a97214f1dc429e Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Thu, 21 Oct 2021 12:29:11 -0400 Subject: [PATCH 2/2] fix(@angular-devkit/build-angular): remove potential race condition in i18n worker execution There was previously the potential for two workers to complete quickly at the same time which could result in one of the results not being propagated to the remainder of the system. This situation has now been corrected by removing the worker execution at a later point in the process. --- .../build_angular/src/utils/action-executor.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/utils/action-executor.ts b/packages/angular_devkit/build_angular/src/utils/action-executor.ts index d7548192e060..4daf9dd6b823 100644 --- a/packages/angular_devkit/build_angular/src/utils/action-executor.ts +++ b/packages/angular_devkit/build_angular/src/utils/action-executor.ts @@ -77,21 +77,19 @@ export class BundleActionExecutor { actions: Iterable, executor: (action: I) => Promise, ): AsyncIterable { - const executions = new Map, Promise>(); + const executions = new Map, Promise<[Promise, O]>>(); for (const action of actions) { const execution = executor(action); executions.set( execution, - execution.then((result) => { - executions.delete(execution); - - return result; - }), + execution.then((result) => [execution, result]), ); } while (executions.size > 0) { - yield Promise.race(executions.values()); + const [execution, result] = await Promise.race(executions.values()); + executions.delete(execution); + yield result; } }