fix(compiler): remove i18n markup even if no translations (#17999)

Fixes #11042

Author: vicb

packages/compiler-cli/integrationtest/alt/src/bootstrap.ts

@@ -0,0 +1,12 @@
+/**
+ * @license
+ * Copyright Google Inc. 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 {BasicComp} from '../../src/basic';
+import {MainModuleNgFactory} from './module.ngfactory';
+
+MainModuleNgFactory.create(null).instance.appRef.bootstrap(BasicComp);

packages/compiler-cli/integrationtest/test/basic_spec.ts

@@ -12,6 +12,7 @@ import * as path from 'path';
 import {MultipleComponentsMyComp} from '../src/a/multiple_components';
 import {BasicComp} from '../src/basic';
 import {createComponent} from './util';
+import {createComponentAlt} from './util_alt';
 
 describe('template codegen output', () => {
   const outDir = 'src';
@@ -88,5 +89,17 @@ describe('template codegen output', () => {
       const pText = pElement.children.map((c: any) => c.data).join('').trim();
       expect(pText).toBe('tervetuloa');
     });
+
+    it('should have removed i18n markup', () => {
+      const containerElement = createComponent(BasicComp).debugElement.children[0];
+      expect(containerElement.attributes['title']).toBe('käännä teksti');
+      expect(containerElement.attributes['i18n-title']).toBeUndefined();
+    });
+
+    it('should have removed i18n markup event without translations', () => {
+      const containerElement = createComponentAlt(BasicComp).debugElement.children[0];
+      expect(containerElement.attributes['title']).toBe('translate me');
+      expect(containerElement.attributes['i18n-title']).toBeUndefined();
+    });
   });
 });

packages/compiler-cli/integrationtest/test/util_alt.ts

@@ -0,0 +1,33 @@
+/**
+ * @license
+ * Copyright Google Inc. 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 {NgModuleRef} from '@angular/core';
+import {ComponentFixture} from '@angular/core/testing';
+import {platformServerTesting} from '@angular/platform-server/testing';
+
+import {MainModuleNgFactory} from '../alt/src/module.ngfactory';
+import {MainModule} from '../src/module';
+
+let mainModuleRef: NgModuleRef<MainModule> = null !;
+beforeEach((done) => {
+  platformServerTesting().bootstrapModuleFactory(MainModuleNgFactory).then((moduleRef: any) => {
+    mainModuleRef = moduleRef;
+    done();
+  });
+});
+
+export function createModule(): NgModuleRef<MainModule> {
+  return mainModuleRef;
+}
+
+export function createComponentAlt<C>(comp: {new (...args: any[]): C}): ComponentFixture<C> {
+  const moduleRef = createModule();
+  const compRef =
+      moduleRef.componentFactoryResolver.resolveComponentFactory(comp).create(moduleRef.injector);
+  return new ComponentFixture(compRef, null, false);
+}

packages/compiler-cli/integrationtest/tsconfig-build-alt.json

@@ -0,0 +1,32 @@
+{
+  "angularCompilerOptions": {
+    // For TypeScript 1.8, we have to lay out generated files
+    // in the same source directory with your code.
+    "genDir": "./alt",
+    "debug": true,
+    "enableSummariesForJit": true,
+    "alwaysCompileGeneratedCode": true
+  },
+
+  "compilerOptions": {
+    "target": "es5",
+    "experimentalDecorators": true,
+    "noImplicitAny": true,
+    "strictNullChecks": true,
+    "skipLibCheck": true,
+    "moduleResolution": "node",
+    "rootDir": "",
+    "declaration": true,
+    "lib": ["es6", "dom"],
+    "baseUrl": ".",
+    // Prevent scanning up the directory tree for types
+    "typeRoots": ["node_modules/@types"],
+    "noUnusedLocals": true,
+    "sourceMap": true
+  },
+
+  "files": [
+    "src/module",
+    "alt/src/bootstrap"
+  ]
+}

packages/compiler-cli/src/codegen.ts

@@ -95,6 +95,9 @@ export class CodeGenerator {
               `Unknown option for missingTranslation (${cliOptions.missingTranslation}). Use either error, warning or ignore.`);
       }
     }
+    if (!transContent) {
+      missingTranslation = MissingTranslationStrategy.Ignore
+    }
     const {compiler: aotCompiler} = compiler.createAotCompiler(ngCompilerHost, {
       translations: transContent,
       i18nFormat: cliOptions.i18nFormat,

packages/compiler/src/i18n/extractor.ts

@@ -29,8 +29,6 @@ import {ParseError} from '../parse_util';
 import {PipeResolver} from '../pipe_resolver';
 import {DomElementSchemaRegistry} from '../schema/dom_element_schema_registry';
 import {createOfflineCompileUrlResolver} from '../url_resolver';
-
-import {I18NHtmlParser} from './i18n_html_parser';
 import {MessageBundle} from './message_bundle';
 
 /**
@@ -87,7 +85,7 @@ export class Extractor {
 
   static create(host: ExtractorHost, locale: string|null):
       {extractor: Extractor, staticReflector: StaticReflector} {
-    const htmlParser = new I18NHtmlParser(new HtmlParser());
+    const htmlParser = new HtmlParser();
 
     const urlResolver = createOfflineCompileUrlResolver();
     const symbolCache = new StaticSymbolCache();

packages/compiler/src/i18n/i18n_html_parser.ts

@@ -7,9 +7,12 @@
  */
 
 import {MissingTranslationStrategy, ɵConsole as Console} from '@angular/core';
+
 import {HtmlParser} from '../ml_parser/html_parser';
 import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config';
 import {ParseTreeResult} from '../ml_parser/parser';
+
+import {digest} from './digest';
 import {mergeTranslations} from './extractor_merger';
 import {Serializer} from './serializers/serializer';
 import {Xliff} from './serializers/xliff';
@@ -32,6 +35,9 @@ export class I18NHtmlParser implements HtmlParser {
       const serializer = createSerializer(translationsFormat);
       this._translationBundle =
           TranslationBundle.load(translations, 'i18n', serializer, missingTranslation, console);
+    } else {
+      this._translationBundle =
+          new TranslationBundle({}, null, digest, undefined, missingTranslation, console);
     }
   }
 
@@ -41,11 +47,6 @@ export class I18NHtmlParser implements HtmlParser {
     const parseResult =
         this._htmlParser.parse(source, url, parseExpansionForms, interpolationConfig);
 
-    if (!this._translationBundle) {
-      // Do not enable i18n when no translation bundle is provided
-      return parseResult;
-    }
-
     if (parseResult.errors.length) {
       return new ParseTreeResult(parseResult.rootNodes, parseResult.errors);
     }

packages/compiler/src/jit/compiler_factory.ts

@@ -59,10 +59,13 @@ export const COMPILER_PROVIDERS: Array<any|Type<any>|{[k: string]: any}|any[]> =
   },
   {
     provide: i18n.I18NHtmlParser,
-    useFactory: (parser: HtmlParser, translations: string, format: string, config: CompilerConfig,
-                 console: Console) =>
-                    new i18n.I18NHtmlParser(
-                        parser, translations, format, config.missingTranslation !, console),
+    useFactory: (parser: HtmlParser, translations: string | null, format: string,
+                 config: CompilerConfig, console: Console) => {
+      translations = translations || '';
+      const missingTranslation =
+          translations ? config.missingTranslation ! : MissingTranslationStrategy.Ignore;
+      return new i18n.I18NHtmlParser(parser, translations, format, missingTranslation, console);
+    },
     deps: [
       baseHtmlParser,
       [new Optional(), new Inject(TRANSLATIONS)],

packages/compiler/test/i18n/extractor_merger_spec.ts

@@ -7,6 +7,7 @@
  */
 
 import {DEFAULT_INTERPOLATION_CONFIG, HtmlParser} from '@angular/compiler';
+import {MissingTranslationStrategy} from '@angular/core';
 
 import {digest, serializeNodes as serializeI18nNodes} from '../../src/i18n/digest';
 import {extractMessages, mergeTranslations} from '../../src/i18n/extractor_merger';
@@ -465,6 +466,31 @@ export function main() {
             .toEqual(`<div title="">some element</div>`);
       });
     });
+
+    describe('no translations', () => {
+      it('should remove i18n attributes', () => {
+        const HTML = `<p i18n="m|d">foo</p>`;
+        expect(fakeNoTranslate(HTML)).toEqual('<p>foo</p>');
+      });
+
+      it('should remove i18n- attributes', () => {
+        const HTML = `<p i18n-title="m|d" title="foo"></p>`;
+        expect(fakeNoTranslate(HTML)).toEqual('<p title="foo"></p>');
+      });
+
+      it('should remove i18n comment blocks', () => {
+        const HTML = `before<!-- i18n --><p>foo</p><span><i>bar</i></span><!-- /i18n -->after`;
+        expect(fakeNoTranslate(HTML)).toEqual('before<p>foo</p><span><i>bar</i></span>after');
+      });
+
+      it('should remove nested i18n markup', () => {
+        const HTML =
+            `<!-- i18n --><span someAttr="ok">foo</span><div>{count, plural, =0 {<p i18n-title title="foo"></p>}}</div><!-- /i18n -->`;
+        expect(fakeNoTranslate(HTML))
+            .toEqual(
+                '<span someAttr="ok">foo</span><div>{count, plural, =0 {<p title="foo"></p>}}</div>');
+      });
+    })
   });
 }
 
@@ -493,10 +519,22 @@ function fakeTranslate(
     i18nMsgMap[id] = [new i18n.Text(`**${text}**`, null !)];
   });
 
-  const translations = new TranslationBundle(i18nMsgMap, null, digest);
+  const translationBundle = new TranslationBundle(i18nMsgMap, null, digest);
+  const output = mergeTranslations(
+      htmlNodes, translationBundle, DEFAULT_INTERPOLATION_CONFIG, implicitTags, implicitAttrs);
+  expect(output.errors).toEqual([]);
+
+  return serializeHtmlNodes(output.rootNodes).join('');
+}
 
+function fakeNoTranslate(
+    content: string, implicitTags: string[] = [],
+    implicitAttrs: {[k: string]: string[]} = {}): string {
+  const htmlNodes: html.Node[] = parseHtml(content);
+  const translationBundle = new TranslationBundle(
+      {}, null, digest, undefined, MissingTranslationStrategy.Ignore, console);
   const output = mergeTranslations(
-      htmlNodes, translations, DEFAULT_INTERPOLATION_CONFIG, implicitTags, implicitAttrs);
+      htmlNodes, translationBundle, DEFAULT_INTERPOLATION_CONFIG, implicitTags, implicitAttrs);
   expect(output.errors).toEqual([]);
 
   return serializeHtmlNodes(output.rootNodes).join('');

packages/compiler/test/i18n/i18n_html_parser_spec.ts

@@ -13,22 +13,6 @@ import {ParseTreeResult} from '@angular/compiler/src/ml_parser/parser';
 
 export function main() {
   describe('I18N html parser', () => {
-
-    it('should return the html nodes when no translations are given', () => {
-      const htmlParser = new HtmlParser();
-      const i18nHtmlParser = new I18NHtmlParser(htmlParser);
-      const ptResult = new ParseTreeResult([], []);
-
-      spyOn(htmlParser, 'parse').and.returnValue(ptResult);
-      spyOn(i18nHtmlParser, 'parse').and.callThrough();
-
-      expect(i18nHtmlParser.parse('source', 'url')).toBe(ptResult);
-
-      expect(htmlParser.parse).toHaveBeenCalledTimes(1);
-      expect(htmlParser.parse)
-          .toHaveBeenCalledWith('source', 'url', jasmine.anything(), jasmine.anything());
-    });
-
     // https://github.com/angular/angular/issues/14322
     it('should parse the translations only once', () => {
       const transBundle = new TranslationBundle({}, null, () => 'id');