Skip to content

Commit 7f4da00

Browse files
clydindgp1130
authored andcommitted
fix(@angular-devkit/build-angular): improve quality of localized sourcemaps
Fixes: #17131 (cherry picked from commit 2e84203)
1 parent 79cd542 commit 7f4da00

File tree

4 files changed

+63
-36
lines changed

4 files changed

+63
-36
lines changed

packages/angular_devkit/build_angular/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
"less-loader": "5.0.0",
3737
"license-webpack-plugin": "2.1.4",
3838
"loader-utils": "2.0.0",
39-
"magic-string": "0.25.7",
4039
"mini-css-extract-plugin": "0.9.0",
4140
"minimatch": "3.0.4",
4241
"parse5": "4.0.0",

packages/angular_devkit/build_angular/src/utils/process-bundle.ts

Lines changed: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,13 @@ import * as path from 'path';
2222
import { RawSourceMap, SourceMapConsumer, SourceMapGenerator } from 'source-map';
2323
import { minify } from 'terser';
2424
import * as v8 from 'v8';
25-
import { SourceMapSource } from 'webpack-sources';
25+
import {
26+
ConcatSource,
27+
OriginalSource,
28+
ReplaceSource,
29+
Source,
30+
SourceMapSource,
31+
} from 'webpack-sources';
2632
import { allowMangle, allowMinify, shouldBeautify } from './environment-options';
2733
import { I18nOptions } from './i18n-options';
2834

@@ -48,7 +54,7 @@ export interface ProcessBundleOptions {
4854
integrityAlgorithm?: 'sha256' | 'sha384' | 'sha512';
4955
runtimeData?: ProcessBundleResult[];
5056
replacements?: [string, string][];
51-
supportedBrowsers?: string [] | Record<string, string>;
57+
supportedBrowsers?: string[] | Record<string, string>;
5258
}
5359

5460
export interface ProcessBundleResult {
@@ -665,7 +671,7 @@ export async function inlineLocales(options: InlineOptions) {
665671
fs.writeFileSync(outputPath, transformResult.code);
666672

667673
if (inputMap && transformResult.map) {
668-
const outputMap = mergeSourceMaps(
674+
const outputMap = await mergeSourceMaps(
669675
options.code,
670676
inputMap,
671677
transformResult.code,
@@ -686,7 +692,6 @@ async function inlineLocalesDirect(ast: ParseResult, options: InlineOptions) {
686692
return { file: options.filename, diagnostics: [], count: 0 };
687693
}
688694

689-
const { default: MagicString } = await import('magic-string');
690695
const { default: generate } = await import('@babel/generator');
691696
const utils = await import(
692697
// tslint:disable-next-line: trailing-comma no-implicit-dependencies
@@ -702,11 +707,21 @@ async function inlineLocalesDirect(ast: ParseResult, options: InlineOptions) {
702707
return inlineCopyOnly(options);
703708
}
704709

705-
// tslint:disable-next-line: no-any
706-
let content = new MagicString(options.code, { filename: options.filename } as any);
707710
const inputMap = options.map && (JSON.parse(options.map) as RawSourceMap);
708-
let contentClone;
711+
// Cleanup source root otherwise it will be added to each source entry
712+
const mapSourceRoot = inputMap && inputMap.sourceRoot;
713+
if (inputMap) {
714+
delete inputMap.sourceRoot;
715+
}
716+
709717
for (const locale of i18n.inlineLocales) {
718+
const content = new ReplaceSource(
719+
inputMap
720+
? // tslint:disable-next-line: no-any
721+
new SourceMapSource(options.code, options.filename, inputMap as any)
722+
: new OriginalSource(options.code, options.filename),
723+
);
724+
710725
const isSourceLocale = locale === i18n.sourceLocale;
711726
// tslint:disable-next-line: no-any
712727
const translations: any = isSourceLocale ? {} : i18n.locales[locale].translation || {};
@@ -722,49 +737,42 @@ async function inlineLocalesDirect(ast: ParseResult, options: InlineOptions) {
722737
const expression = utils.buildLocalizeReplacement(translated[0], translated[1]);
723738
const { code } = generate(expression);
724739

725-
content.overwrite(position.start, position.end, code);
740+
content.replace(position.start, position.end - 1, code);
726741
}
727742

743+
let outputSource: Source = content;
728744
if (options.setLocale) {
729-
const setLocaleText = `var $localize=Object.assign(void 0===$localize?{}:$localize,{locale:"${locale}"});`;
730-
contentClone = content.clone();
731-
content.prepend(setLocaleText);
745+
const setLocaleText = `var $localize=Object.assign(void 0===$localize?{}:$localize,{locale:"${locale}"});\n`;
732746

733747
// If locale data is provided, load it and prepend to file
748+
let localeDataSource: Source | null = null;
734749
const localeDataPath = i18n.locales[locale] && i18n.locales[locale].dataPath;
735750
if (localeDataPath) {
736-
const localDataContent = await loadLocaleData(localeDataPath, true);
737-
// The semicolon ensures that there is no syntax error between statements
738-
content.prepend(localDataContent + ';');
751+
const localeDataContent = await loadLocaleData(localeDataPath, true);
752+
localeDataSource = new OriginalSource(localeDataContent, path.basename(localeDataPath));
739753
}
754+
755+
outputSource = localeDataSource
756+
// The semicolon ensures that there is no syntax error between statements
757+
? new ConcatSource(setLocaleText, localeDataSource, ';\n', content)
758+
: new ConcatSource(setLocaleText, content);
740759
}
741760

742-
const output = content.toString();
761+
const { source: outputCode, map: outputMap } = outputSource.sourceAndMap();
743762
const outputPath = path.join(
744763
options.outputPath,
745764
i18n.flatOutput ? '' : locale,
746765
options.filename,
747766
);
748-
fs.writeFileSync(outputPath, output);
749-
750-
if (inputMap) {
751-
const contentMap = content.generateMap();
752-
const outputMap = mergeSourceMaps(
753-
options.code,
754-
inputMap,
755-
output,
756-
contentMap,
757-
options.filename,
758-
options.code.length > FAST_SOURCEMAP_THRESHOLD,
759-
);
767+
fs.writeFileSync(outputPath, outputCode);
760768

769+
if (inputMap && outputMap) {
770+
outputMap.file = options.filename;
771+
if (mapSourceRoot) {
772+
outputMap.sourceRoot = mapSourceRoot;
773+
}
761774
fs.writeFileSync(outputPath + '.map', JSON.stringify(outputMap));
762775
}
763-
764-
if (contentClone) {
765-
content = contentClone;
766-
contentClone = undefined;
767-
}
768776
}
769777

770778
return { file: options.filename, diagnostics: diagnostics.messages, count: positions.length };

tests/legacy-cli/e2e/tests/i18n/ivy-localize-es2015.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { expectFileNotToExist, expectFileToMatch, writeFile } from '../../utils/fs';
1+
import { expectFileNotToExist, expectFileToMatch, readFile, writeFile } from '../../utils/fs';
22
import { ng } from '../../utils/process';
33
import { updateJsonFile } from '../../utils/project';
44
import { expectToFail } from '../../utils/utils';
@@ -15,12 +15,22 @@ export default async function() {
1515
config.angularCompilerOptions.disableTypeScriptVersionCheck = true;
1616
});
1717

18-
await ng('build');
18+
await ng('build', '--source-map');
1919
for (const { lang, outputPath, translation } of langTranslations) {
2020
await expectFileToMatch(`${outputPath}/main.js`, translation.helloPartial);
2121
await expectToFail(() => expectFileToMatch(`${outputPath}/main.js`, '$localize`'));
2222
await expectFileNotToExist(`${outputPath}/main-es5.js`);
2323

24+
// Ensure sourcemap for modified file contains content
25+
const mainSourceMap = JSON.parse(await readFile(`${outputPath}/main.js.map`));
26+
if (
27+
mainSourceMap.version !== 3 ||
28+
!Array.isArray(mainSourceMap.sources) ||
29+
typeof mainSourceMap.mappings !== 'string'
30+
) {
31+
throw new Error('invalid localized sourcemap for main.js');
32+
}
33+
2434
// Ensure locale is inlined (@angular/localize plugin inlines `$localize.locale` references)
2535
// The only reference in a new application is in @angular/core
2636
await expectFileToMatch(`${outputPath}/vendor.js`, lang);

tests/legacy-cli/e2e/tests/i18n/ivy-localize-es5.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { expectFileNotToExist, expectFileToMatch } from '../../utils/fs';
1+
import { expectFileNotToExist, expectFileToMatch, readFile } from '../../utils/fs';
22
import { ng } from '../../utils/process';
33
import { updateJsonFile } from '../../utils/project';
44
import { expectToFail } from '../../utils/utils';
@@ -21,6 +21,16 @@ export default async function() {
2121
await expectToFail(() => expectFileToMatch(`${outputPath}/main.js`, '$localize`'));
2222
await expectFileNotToExist(`${outputPath}/main-es2015.js`);
2323

24+
// Ensure sourcemap for modified file contains content
25+
const mainSourceMap = JSON.parse(await readFile(`${outputPath}/main.js.map`));
26+
if (
27+
mainSourceMap.version !== 3 ||
28+
!Array.isArray(mainSourceMap.sources) ||
29+
typeof mainSourceMap.mappings !== 'string'
30+
) {
31+
throw new Error('invalid localized sourcemap for main.js');
32+
}
33+
2434
// Ensure locale is inlined (@angular/localize plugin inlines `$localize.locale` references)
2535
// The only reference in a new application is in @angular/core
2636
await expectFileToMatch(`${outputPath}/vendor.js`, lang);

0 commit comments

Comments
 (0)