Skip to content

Commit 0d68ed5

Browse files
alan-agius4dgp1130
alan-agius4
authored and
dgp1130
committed
fix(@angular-devkit/build-angular): localized bundle generation fails in watch mode
Previously, we used to delete the temporary emitted JS and map files. However this causes a problem in watch mode, as Webpack will not re-emit these deleted files during the next incremental re-build. With this change we now delete the entire temporary directory when the process is being terminated instead of a file by file bases. Closes #22395
1 parent 08152f7 commit 0d68ed5

File tree

5 files changed

+135
-27
lines changed

5 files changed

+135
-27
lines changed

‎packages/angular_devkit/build_angular/src/builders/browser/index.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,12 @@ export type BrowserBuilderOutput = json.JsonObject &
7474
outputPath: string;
7575
};
7676

77+
/**
78+
* Maximum time in milliseconds for single build/rebuild
79+
* This accounts for CI variability.
80+
*/
81+
export const BUILD_TIMEOUT = 30_000;
82+
7783
async function initialize(
7884
options: BrowserBuilderSchema,
7985
context: BuilderContext,

‎packages/angular_devkit/build_angular/src/builders/browser/tests/behavior/localize_watch_spec.ts

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import { concatMap, count, take, timeout } from 'rxjs/operators';
10+
import { BUILD_TIMEOUT, buildWebpackBrowser } from '../../index';
11+
import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup';
12+
13+
describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
14+
describe('Behavior: "localize works in watch mode"', () => {
15+
beforeEach(() => {
16+
harness.useProject('test', {
17+
root: '.',
18+
sourceRoot: 'src',
19+
cli: {
20+
cache: {
21+
enabled: false,
22+
},
23+
},
24+
i18n: {
25+
locales: {
26+
'fr': 'src/locales/messages.fr.xlf',
27+
},
28+
},
29+
});
30+
});
31+
32+
it('localize works in watch mode', async () => {
33+
harness.useTarget('build', {
34+
...BASE_OPTIONS,
35+
watch: true,
36+
localize: true,
37+
});
38+
39+
await harness.writeFile(
40+
'src/app/app.component.html',
41+
`
42+
<p id="hello" i18n="An introduction header for this sample">Hello {{ title }}! </p>
43+
`,
44+
);
45+
46+
await harness.writeFile('src/locales/messages.fr.xlf', TRANSLATION_FILE_CONTENT);
47+
48+
const buildCount = await harness
49+
.execute()
50+
.pipe(
51+
timeout(BUILD_TIMEOUT),
52+
concatMap(async ({ result }, index) => {
53+
expect(result?.success).toBe(true);
54+
55+
switch (index) {
56+
case 0: {
57+
harness.expectFile('dist/fr/main.js').content.toContain('Bonjour');
58+
59+
// Trigger rebuild
60+
await harness.appendToFile('src/app/app.component.html', '\n\n');
61+
break;
62+
}
63+
case 1: {
64+
harness.expectFile('dist/fr/main.js').content.toContain('Bonjour');
65+
break;
66+
}
67+
}
68+
}),
69+
take(2),
70+
count(),
71+
)
72+
.toPromise();
73+
74+
expect(buildCount).toBe(2);
75+
});
76+
});
77+
});
78+
79+
const TRANSLATION_FILE_CONTENT = `
80+
<?xml version="1.0" encoding="UTF-8" ?>
81+
<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">
82+
<file target-language="en-US" datatype="plaintext" original="ng2.template">
83+
<body>
84+
<trans-unit id="4286451273117902052" datatype="html">
85+
<target>Bonjour <x id="INTERPOLATION" equiv-text="{{ title }}"/>! </target>
86+
<context-group purpose="location">
87+
<context context-type="targetfile">src/app/app.component.html</context>
88+
<context context-type="linenumber">2,3</context>
89+
</context-group>
90+
<note priority="1" from="description">An introduction header for this sample</note>
91+
</trans-unit>
92+
</body>
93+
</file>
94+
</xliff>
95+
`;

‎packages/angular_devkit/build_angular/src/builders/browser/tests/behavior/rebuild-errors_spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import { logging } from '@angular-devkit/core';
1010
import { concatMap, count, take, timeout } from 'rxjs/operators';
11-
import { buildWebpackBrowser } from '../../index';
11+
import { BUILD_TIMEOUT, buildWebpackBrowser } from '../../index';
1212
import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup';
1313

1414
describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
@@ -70,7 +70,7 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
7070
const buildCount = await harness
7171
.execute({ outputLogsOnFailure: false })
7272
.pipe(
73-
timeout(60000),
73+
timeout(BUILD_TIMEOUT),
7474
concatMap(async ({ result, logs }, index) => {
7575
switch (index) {
7676
case 0:
@@ -219,7 +219,7 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
219219
const buildCount = await harness
220220
.execute({ outputLogsOnFailure: false })
221221
.pipe(
222-
timeout(60000),
222+
timeout(BUILD_TIMEOUT),
223223
concatMap(async ({ result, logs }, index) => {
224224
switch (index) {
225225
case 0:
@@ -307,7 +307,7 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
307307
const buildCount = await harness
308308
.execute({ outputLogsOnFailure: false })
309309
.pipe(
310-
timeout(30000),
310+
timeout(BUILD_TIMEOUT),
311311
concatMap(async ({ result, logs }, index) => {
312312
switch (index) {
313313
case 0:

‎packages/angular_devkit/build_angular/src/utils/i18n-inlining.ts

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -47,28 +47,10 @@ function emittedFilesToInlineOptions(
4747
};
4848
originalFiles.push(originalPath);
4949

50-
// Remove temporary original file as the content has now been read
51-
try {
52-
fs.unlinkSync(originalPath);
53-
} catch (e) {
54-
context.logger.debug(
55-
`Unable to delete i18n temporary file [${originalPath}]: ${e.toString()}`,
56-
);
57-
}
58-
5950
try {
6051
const originalMapPath = originalPath + '.map';
6152
action.map = fs.readFileSync(originalMapPath, 'utf8');
6253
originalFiles.push(originalMapPath);
63-
64-
// Remove temporary original map file as the content has now been read
65-
try {
66-
fs.unlinkSync(originalMapPath);
67-
} catch (e) {
68-
context.logger.debug(
69-
`Unable to delete i18n temporary file [${originalMapPath}]: ${e.toString()}`,
70-
);
71-
}
7254
} catch (err) {
7355
if (err.code !== 'ENOENT') {
7456
throw err;

‎packages/angular_devkit/build_angular/src/utils/i18n-options.ts

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,11 +249,12 @@ export async function configureI18nBuild<T extends BrowserBuilderSchema | Server
249249
const tempPath = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), 'angular-cli-i18n-'));
250250
buildOptions.outputPath = tempPath;
251251

252-
// Remove temporary directory used for i18n processing
253-
process.on('exit', () => {
254-
try {
255-
fs.rmdirSync(tempPath, { recursive: true, maxRetries: 3 });
256-
} catch {}
252+
process.on('exit', () => deleteTempDirectory(tempPath));
253+
process.once('SIGINT', () => {
254+
deleteTempDirectory(tempPath);
255+
256+
// Needed due to `ora` as otherwise process will not terminate.
257+
process.kill(process.pid, 'SIGINT');
257258
});
258259
}
259260

@@ -276,6 +277,30 @@ function findLocaleDataPath(locale: string, resolver: (locale: string) => string
276277
}
277278
}
278279

280+
/** Remove temporary directory used for i18n processing. */
281+
function deleteTempDirectory(tempPath: string): void {
282+
// The below should be removed and replaced with just `rmSync` when support for Node.Js 12 is removed.
283+
const { rmSync, rmdirSync } = fs as typeof fs & {
284+
rmSync?: (
285+
path: fs.PathLike,
286+
options?: {
287+
force?: boolean;
288+
maxRetries?: number;
289+
recursive?: boolean;
290+
retryDelay?: number;
291+
},
292+
) => void;
293+
};
294+
295+
try {
296+
if (rmSync) {
297+
rmSync(tempPath, { force: true, recursive: true, maxRetries: 3 });
298+
} else {
299+
rmdirSync(tempPath, { recursive: true, maxRetries: 3 });
300+
}
301+
} catch {}
302+
}
303+
279304
export function loadTranslations(
280305
locale: string,
281306
desc: LocaleDescription,

0 commit comments

Comments
 (0)