Skip to content

Commit d674dcd

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 (cherry picked from commit 0d68ed5)
1 parent de5fe42 commit d674dcd

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
@@ -248,11 +248,12 @@ export async function configureI18nBuild<T extends BrowserBuilderSchema | Server
248248
const tempPath = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), 'angular-cli-i18n-'));
249249
buildOptions.outputPath = tempPath;
250250

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

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

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

0 commit comments

Comments
 (0)