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)
angular · Jan 12, 2022 · d674dcd · d674dcd
1 parent de5fe42
commit d674dcd
Show file tree

Hide file tree

Showing 5 changed files with 135 additions and 27 deletions.
diff --git a/packages/angular_devkit/build_angular/src/builders/browser/index.ts b/packages/angular_devkit/build_angular/src/builders/browser/index.ts
@@ -74,6 +74,12 @@ export type BrowserBuilderOutput = json.JsonObject &
     outputPath: string;
   };
 
+/**
+ * Maximum time in milliseconds for single build/rebuild
+ * This accounts for CI variability.
+ */
+export const BUILD_TIMEOUT = 30_000;
+
 async function initialize(
   options: BrowserBuilderSchema,
   context: BuilderContext,

diff --git a/...s/angular_devkit/build_angular/src/builders/browser/tests/behavior/localize_watch_spec.ts b/...s/angular_devkit/build_angular/src/builders/browser/tests/behavior/localize_watch_spec.ts
@@ -0,0 +1,95 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import { concatMap, count, take, timeout } from 'rxjs/operators';
+import { BUILD_TIMEOUT, buildWebpackBrowser } from '../../index';
+import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup';
+
+describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
+  describe('Behavior: "localize works in watch mode"', () => {
+    beforeEach(() => {
+      harness.useProject('test', {
+        root: '.',
+        sourceRoot: 'src',
+        cli: {
+          cache: {
+            enabled: false,
+          },
+        },
+        i18n: {
+          locales: {
+            'fr': 'src/locales/messages.fr.xlf',
+          },
+        },
+      });
+    });
+
+    it('localize works in watch mode', async () => {
+      harness.useTarget('build', {
+        ...BASE_OPTIONS,
+        watch: true,
+        localize: true,
+      });
+
+      await harness.writeFile(
+        'src/app/app.component.html',
+        `
+          <p id="hello" i18n="An introduction header for this sample">Hello {{ title }}! </p>
+        `,
+      );
+
+      await harness.writeFile('src/locales/messages.fr.xlf', TRANSLATION_FILE_CONTENT);
+
+      const buildCount = await harness
+        .execute()
+        .pipe(
+          timeout(BUILD_TIMEOUT),
+          concatMap(async ({ result }, index) => {
+            expect(result?.success).toBe(true);
+
+            switch (index) {
+              case 0: {
+                harness.expectFile('dist/fr/main.js').content.toContain('Bonjour');
+
+                // Trigger rebuild
+                await harness.appendToFile('src/app/app.component.html', '\n\n');
+                break;
+              }
+              case 1: {
+                harness.expectFile('dist/fr/main.js').content.toContain('Bonjour');
+                break;
+              }
+            }
+          }),
+          take(2),
+          count(),
+        )
+        .toPromise();
+
+      expect(buildCount).toBe(2);
+    });
+  });
+});
+
+const TRANSLATION_FILE_CONTENT = `
+  <?xml version="1.0" encoding="UTF-8" ?>
+  <xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">
+    <file target-language="en-US" datatype="plaintext" original="ng2.template">
+      <body>
+        <trans-unit id="4286451273117902052" datatype="html">
+          <target>Bonjour <x id="INTERPOLATION" equiv-text="{{ title }}"/>! </target>
+          <context-group purpose="location">
+            <context context-type="targetfile">src/app/app.component.html</context>
+            <context context-type="linenumber">2,3</context>
+          </context-group>
+          <note priority="1" from="description">An introduction header for this sample</note>
+        </trans-unit>
+      </body>
+    </file>
+  </xliff>
+`;
diff --git a/...s/angular_devkit/build_angular/src/builders/browser/tests/behavior/rebuild-errors_spec.ts b/...s/angular_devkit/build_angular/src/builders/browser/tests/behavior/rebuild-errors_spec.ts
@@ -8,7 +8,7 @@
 
 import { logging } from '@angular-devkit/core';
 import { concatMap, count, take, timeout } from 'rxjs/operators';
-import { buildWebpackBrowser } from '../../index';
+import { BUILD_TIMEOUT, buildWebpackBrowser } from '../../index';
 import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup';
 
 describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
@@ -70,7 +70,7 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
       const buildCount = await harness
         .execute({ outputLogsOnFailure: false })
         .pipe(
-          timeout(60000),
+          timeout(BUILD_TIMEOUT),
           concatMap(async ({ result, logs }, index) => {
             switch (index) {
               case 0:
@@ -219,7 +219,7 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
       const buildCount = await harness
         .execute({ outputLogsOnFailure: false })
         .pipe(
-          timeout(60000),
+          timeout(BUILD_TIMEOUT),
           concatMap(async ({ result, logs }, index) => {
             switch (index) {
               case 0:
@@ -307,7 +307,7 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
       const buildCount = await harness
         .execute({ outputLogsOnFailure: false })
         .pipe(
-          timeout(30000),
+          timeout(BUILD_TIMEOUT),
           concatMap(async ({ result, logs }, index) => {
             switch (index) {
               case 0:

diff --git a/packages/angular_devkit/build_angular/src/utils/i18n-inlining.ts b/packages/angular_devkit/build_angular/src/utils/i18n-inlining.ts
@@ -47,28 +47,10 @@ function emittedFilesToInlineOptions(
     };
     originalFiles.push(originalPath);
 
-    // Remove temporary original file as the content has now been read
-    try {
-      fs.unlinkSync(originalPath);
-    } catch (e) {
-      context.logger.debug(
-        `Unable to delete i18n temporary file [${originalPath}]: ${e.toString()}`,
-      );
-    }
-
     try {
       const originalMapPath = originalPath + '.map';
       action.map = fs.readFileSync(originalMapPath, 'utf8');
       originalFiles.push(originalMapPath);
-
-      // Remove temporary original map file as the content has now been read
-      try {
-        fs.unlinkSync(originalMapPath);
-      } catch (e) {
-        context.logger.debug(
-          `Unable to delete i18n temporary file [${originalMapPath}]: ${e.toString()}`,
-        );
-      }
     } catch (err) {
       if (err.code !== 'ENOENT') {
         throw err;

diff --git a/packages/angular_devkit/build_angular/src/utils/i18n-options.ts b/packages/angular_devkit/build_angular/src/utils/i18n-options.ts
@@ -248,11 +248,12 @@ export async function configureI18nBuild<T extends BrowserBuilderSchema | Server
     const tempPath = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), 'angular-cli-i18n-'));
     buildOptions.outputPath = tempPath;
 
-    // Remove temporary directory used for i18n processing
-    process.on('exit', () => {
-      try {
-        fs.rmdirSync(tempPath, { recursive: true, maxRetries: 3 });
-      } catch {}
+    process.on('exit', () => deleteTempDirectory(tempPath));
+    process.once('SIGINT', () => {
+      deleteTempDirectory(tempPath);
+
+      // Needed due to `ora` as otherwise process will not terminate.
+      process.kill(process.pid, 'SIGINT');
     });
   }
 
@@ -275,6 +276,30 @@ function findLocaleDataPath(locale: string, resolver: (locale: string) => string
   }
 }
 
+/** Remove temporary directory used for i18n processing. */
+function deleteTempDirectory(tempPath: string): void {
+  // The below should be removed and replaced with just `rmSync` when support for Node.Js 12 is removed.
+  const { rmSync, rmdirSync } = fs as typeof fs & {
+    rmSync?: (
+      path: fs.PathLike,
+      options?: {
+        force?: boolean;
+        maxRetries?: number;
+        recursive?: boolean;
+        retryDelay?: number;
+      },
+    ) => void;
+  };
+
+  try {
+    if (rmSync) {
+      rmSync(tempPath, { force: true, recursive: true, maxRetries: 3 });
+    } else {
+      rmdirSync(tempPath, { recursive: true, maxRetries: 3 });
+    }
+  } catch {}
+}
+
 export function loadTranslations(
   locale: string,
   desc: LocaleDescription,