refactor(compiler): add a version to the i18n serializers

fixes #15573 `I18nVersion.Version1` fixes several bugs in the serializers. `I18nVersion.Version0` is identical to the behavior prior to this PR.
ocombe · May 17, 2017 · Jun 7, 2017 · Jun 8, 2017 · Jun 8, 2017 · Jun 8, 2017
commit 12d930c9548104a9808280d78cb7e7aea8a98324
diff --git a/packages/compiler-cli/src/extractor.ts b/packages/compiler-cli/src/extractor.ts
@@ -14,6 +14,7 @@
 import 'reflect-metadata';
 
 import * as compiler from '@angular/compiler';
+import {I18nVersion} from '@angular/core';
 import * as tsc from '@angular/tsc-wrapped';
 import * as path from 'path';
 import * as ts from 'typescript';
@@ -55,16 +56,16 @@ export class Extractor {
 
     switch (format) {
       case 'xmb':
-        serializer = new compiler.Xmb();
+        serializer = new compiler.Xmb(I18nVersion.V0);
         break;
       case 'xliff2':
       case 'xlf2':
-        serializer = new compiler.Xliff2();
+        serializer = new compiler.Xliff2(I18nVersion.V0);
         break;
       case 'xlf':
       case 'xliff':
       default:
-        serializer = new compiler.Xliff();
+        serializer = new compiler.Xliff(I18nVersion.V0);
     }
     return bundle.write(
         serializer, (sourcePath: string) => this.options.basePath ?

diff --git a/packages/compiler-cli/test/diagnostics/mocks.ts b/packages/compiler-cli/test/diagnostics/mocks.ts
@@ -7,7 +7,7 @@
  */
 
 import {AotCompilerHost, AotSummaryResolver, CompileMetadataResolver, CompilerConfig, DEFAULT_INTERPOLATION_CONFIG, DirectiveNormalizer, DirectiveResolver, DomElementSchemaRegistry, HtmlParser, I18NHtmlParser, InterpolationConfig, JitSummaryResolver, Lexer, NgAnalyzedModules, NgModuleResolver, ParseTreeResult, Parser, PipeResolver, ResourceLoader, StaticReflector, StaticSymbol, StaticSymbolCache, StaticSymbolResolver, SummaryResolver, TemplateParser, analyzeNgModules, createOfflineCompileUrlResolver, extractProgramSymbols} from '@angular/compiler';
-import {ViewEncapsulation, ɵConsole as Console} from '@angular/core';
+import {I18nVersion, ViewEncapsulation, ɵConsole as Console} from '@angular/core';
 import {CompilerHostContext} from 'compiler-cli';
 import * as fs from 'fs';
 import * as path from 'path';
@@ -204,7 +204,7 @@ function compileTemplate(context: DiagnosticContext, type: StaticSymbol, templat
   const metadata = resolvedMetadata && resolvedMetadata.metadata;
   if (metadata) {
     const rawHtmlParser = new HtmlParser();
-    const htmlParser = new I18NHtmlParser(rawHtmlParser);
+    const htmlParser = new I18NHtmlParser(rawHtmlParser, I18nVersion.V0);
     const expressionParser = new Parser(new Lexer());
     const config = new CompilerConfig();
     const parser = new TemplateParser(

diff --git a/packages/compiler/src/aot/compiler_factory.ts b/packages/compiler/src/aot/compiler_factory.ts
@@ -6,7 +6,8 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {MissingTranslationStrategy, ViewEncapsulation, ɵConsole as Console} from '@angular/core';
+import {I18nVersion, MissingTranslationStrategy, ViewEncapsulation, ɵConsole as Console} from '@angular/core';
+
 import {CompilerConfig} from '../config';
 import {DirectiveNormalizer} from '../directive_normalizer';
 import {DirectiveResolver} from '../directive_resolver';
@@ -34,6 +35,7 @@ import {StaticSymbolResolver} from './static_symbol_resolver';
 import {AotSummaryResolver} from './summary_resolver';
 
 
+
 /**
  * Creates a new AotCompiler based on options and a host.
  */
@@ -47,8 +49,10 @@ export function createAotCompiler(compilerHost: AotCompilerHost, options: AotCom
   const symbolResolver = new StaticSymbolResolver(compilerHost, symbolCache, summaryResolver);
   const staticReflector = new StaticReflector(summaryResolver, symbolResolver);
   const console = new Console();
+  const i18nFormat = options.i18nFormat || 'xlf';
   const htmlParser = new I18NHtmlParser(
-      new HtmlParser(), translations, options.i18nFormat, options.missingTranslation, console);
+      new HtmlParser(), I18nVersion.V0, translations, i18nFormat, options.missingTranslation,
+      console);
   const config = new CompilerConfig({
     defaultEncapsulation: ViewEncapsulation.Emulated,
     useJit: false,

diff --git a/packages/compiler/src/i18n/digest.ts b/packages/compiler/src/i18n/digest.ts
@@ -10,16 +10,46 @@ import {utf8Encode} from '../util';
 
 import * as i18n from './i18n_ast';
 
-export function digest(message: i18n.Message): string {
+/**
+ * Return a digest of the message composed of decimal digits.
+ *
+ * @internal
+ */
+export function decimalDigest(message: i18n.Message): string {
+  if (message.id) {
+    return message.id;
+  }
+
+  const visitor = new SerializerIgnorePhVisitor();
+  const parts = message.nodes.map(a => a.visit(visitor, null));
+  return computeMsgId(parts.join(''), message.meaning);
+}
+
+/**
+ * Return a digest of the message using a sha1 algorithm.
+ *
+ * @internal
+ * @deprecated from v5, use `decimalDigest` instead
+ */
+export function sha1Digest(message: i18n.Message): string {
   return message.id || sha1(serializeNodes(message.nodes).join('') + `[${message.meaning}]`);
 }
 
-export function decimalDigest(message: i18n.Message): string {
+/**
+ * Return a digest of the message composed of decimal digits.
+ *
+ * NOTE: This method is deprecated and should not be used because the digest depends on the content
+ *       of the interpolation placeholders while it should only depends on their names.
+ *
+ * @internal
+ * @deprecated from v5, use `decimalDigest` instead
+ */
+export function decimalDigestDeprecated(message: i18n.Message): string {
   if (message.id) {
     return message.id;
   }
 
-  const visitor = new _SerializerIgnoreIcuExpVisitor();
+  const visitor = new SerializerIgnoreIcuExpVisitor();
   const parts = message.nodes.map(a => a.visit(visitor, null));
   return computeMsgId(parts.join(''), message.meaning);
 }
@@ -28,10 +58,8 @@ export function decimalDigest(message: i18n.Message): string {
  * Serialize the i18n ast to something xml-like in order to generate an UID.
  *
  * The visitor is also used in the i18n parser tests
- *
- * @internal
  */
-class _SerializerVisitor implements i18n.Visitor {
+class SerializerVisitor implements i18n.Visitor {
   visitText(text: i18n.Text, context: any): any { return text.value; }
 
   visitContainer(container: i18n.Container, context: any): any {
@@ -59,7 +87,7 @@ class _SerializerVisitor implements i18n.Visitor {
   }
 }
 
-const serializerVisitor = new _SerializerVisitor();
+const serializerVisitor = new SerializerVisitor();
 
 export function serializeNodes(nodes: i18n.Node[]): string[] {
   return nodes.map(a => a.visit(serializerVisitor, null));
@@ -69,17 +97,24 @@ export function serializeNodes(nodes: i18n.Node[]): string[] {
  * Serialize the i18n ast to something xml-like in order to generate an UID.
  *
  * Ignore the ICU expressions so that message IDs stays identical if only the expression changes.
- *
- * @internal
  */
-class _SerializerIgnoreIcuExpVisitor extends _SerializerVisitor {
+class SerializerIgnoreIcuExpVisitor extends SerializerVisitor {
   visitIcu(icu: i18n.Icu, context: any): any {
     let strCases = Object.keys(icu.cases).map((k: string) => `${k} {${icu.cases[k].visit(this)}}`);
     // Do not take the expression into account
     return `{${icu.type}, ${strCases.join(', ')}}`;
   }
 }
 
+/**
+ * Serialize the i18n ast to something xml-like in order to generate an UID.
+ *
+ * Ignore the PH/ICU expressions so that message IDs stays identical if only the expression changes.
+ */
+class SerializerIgnorePhVisitor extends SerializerIgnoreIcuExpVisitor {
+  visitPlaceholder(ph: i18n.Placeholder, context: any): any { return `<ph name="${ph.name}"/>`; }
+}
+
 /**
  * Compute the SHA1 of the given string
  *

diff --git a/packages/compiler/src/i18n/extractor.ts b/packages/compiler/src/i18n/extractor.ts
@@ -10,7 +10,7 @@
 /**
  * Extract i18n messages from source code
  */
-import {ViewEncapsulation, ɵConsole as Console} from '@angular/core';
+import {I18nVersion, ViewEncapsulation, ɵConsole as Console} from '@angular/core';
 
 import {analyzeAndValidateNgModules, extractProgramSymbols} from '../aot/compiler';
 import {StaticReflector} from '../aot/static_reflector';
@@ -29,8 +29,11 @@ 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';
 
+
 /**
  * The host of the Extractor disconnects the implementation from TypeScript / other language
  * services and from underlying file systems.
@@ -85,7 +88,7 @@ export class Extractor {
 
   static create(host: ExtractorHost, locale: string|null):
       {extractor: Extractor, staticReflector: StaticReflector} {
-    const htmlParser = new HtmlParser();
+    const htmlParser = new I18NHtmlParser(new HtmlParser(), I18nVersion.V0);
 
     const urlResolver = createOfflineCompileUrlResolver();
     const symbolCache = new StaticSymbolCache();

diff --git a/packages/compiler/src/i18n/i18n_html_parser.ts b/packages/compiler/src/i18n/i18n_html_parser.ts
@@ -6,13 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {MissingTranslationStrategy, ɵConsole as Console} from '@angular/core';
-
+import {I18nVersion, 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 {sha1Digest} from './digest';
 import {mergeTranslations} from './extractor_merger';
 import {Serializer} from './serializers/serializer';
 import {Xliff} from './serializers/xliff';
@@ -28,16 +26,20 @@ export class I18NHtmlParser implements HtmlParser {
   private _translationBundle: TranslationBundle;
 
   constructor(
-      private _htmlParser: HtmlParser, translations?: string, translationsFormat?: string,
+      private _htmlParser: HtmlParser, version: I18nVersion, translations?: string,
+      translationsFormat?: string,
       missingTranslation: MissingTranslationStrategy = MissingTranslationStrategy.Warning,
       console?: Console) {
     if (translations) {
-      const serializer = createSerializer(translationsFormat);
+      if (!translationsFormat) {
+        throw new Error('The format of the translations should be provided');
+      }
+      const serializer = createSerializer(translationsFormat, version);
       this._translationBundle =
           TranslationBundle.load(translations, 'i18n', serializer, missingTranslation, console);
     } else {
       this._translationBundle =
-          new TranslationBundle({}, null, digest, undefined, missingTranslation, console);
+          new TranslationBundle({}, null, sha1Digest, undefined, missingTranslation, console);
     }
   }
 
@@ -56,20 +58,18 @@ export class I18NHtmlParser implements HtmlParser {
   }
 }
 
-function createSerializer(format?: string): Serializer {
-  format = (format || 'xlf').toLowerCase();
-
+export function createSerializer(format: string, version: I18nVersion): Serializer {
   switch (format) {
     case 'xmb':
-      return new Xmb();
+      return new Xmb(version);
     case 'xtb':
-      return new Xtb();
+      return new Xtb(version);
     case 'xliff2':
     case 'xlf2':
-      return new Xliff2();
+      return new Xliff2(version);
     case 'xliff':
     case 'xlf':
     default:
-      return new Xliff();
+      return new Xliff(version);
   }
 }
diff --git a/packages/compiler/src/i18n/serializers/xliff.ts b/packages/compiler/src/i18n/serializers/xliff.ts
@@ -6,9 +6,12 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {I18nVersion} from '@angular/core';
+
 import * as ml from '../../ml_parser/ast';
 import {XmlParser} from '../../ml_parser/xml_parser';
-import {digest} from '../digest';
+import {ParseError} from '../../parse_util';
+import {decimalDigest, sha1Digest} from '../digest';
 import * as i18n from '../i18n_ast';
 import {I18nError} from '../parse_util';
 
@@ -31,6 +34,8 @@ const _CONTEXT_TAG = 'context';
 // http://docs.oasis-open.org/xliff/v1.2/os/xliff-core.html
 // http://docs.oasis-open.org/xliff/v1.2/xliff-profile-html/xliff-profile-html-1.2.html
 export class Xliff extends Serializer {
+  constructor(private version: I18nVersion) { super(); }
+
   write(messages: i18n.Message[], locale: string|null): string {
     const visitor = new _WriteVisitor();
     const transUnits: xml.Node[] = [];
@@ -90,7 +95,7 @@ export class Xliff extends Serializer {
   }
 
   load(content: string, url: string):
-      {locale: string, i18nNodesByMsgId: {[msgId: string]: i18n.Node[]}} {
+      {locale: string | null, i18nNodesByMsgId: {[msgId: string]: i18n.Node[]}} {
     // xliff to xml nodes
     const xliffParser = new XliffParser();
     const {locale, msgIdToHtml, errors} = xliffParser.parse(content, url);
@@ -109,10 +114,15 @@ export class Xliff extends Serializer {
       throw new Error(`xliff parse errors:\n${errors.join('\n')}`);
     }
 
-    return {locale: locale !, i18nNodesByMsgId};
+    return {locale, i18nNodesByMsgId};
   }
 
-  digest(message: i18n.Message): string { return digest(message); }
+  digest(message: i18n.Message): string {
+    if (this.version === I18nVersion.V0) {
+      return sha1Digest(message);
+    }
+    return decimalDigest(message);
+  }
 }
 
 class _WriteVisitor implements i18n.Visitor {
@@ -171,7 +181,8 @@ class XliffParser implements ml.Visitor {
   private _msgIdToHtml: {[msgId: string]: string};
   private _locale: string|null = null;
 
-  parse(xliff: string, url: string) {
+  parse(xliff: string, url: string):
+      {msgIdToHtml: {[id: string]: string}, errors: ParseError[], locale: string|null} {
     this._unitMlString = null;
     this._msgIdToHtml = {};
 
@@ -233,6 +244,8 @@ class XliffParser implements ml.Visitor {
         // TODO(vicb): assert file structure, xliff version
         // For now only recurse on unhandled nodes
         ml.visitAll(this, element.children, null);
+
+        return element;
     }
   }
 

diff --git a/packages/compiler/src/i18n/serializers/xliff2.ts b/packages/compiler/src/i18n/serializers/xliff2.ts
@@ -6,9 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {I18nVersion} from '@angular/core';
+
 import * as ml from '../../ml_parser/ast';
 import {XmlParser} from '../../ml_parser/xml_parser';
-import {decimalDigest} from '../digest';
+import {decimalDigest, decimalDigestDeprecated} from '../digest';
 import * as i18n from '../i18n_ast';
 import {I18nError} from '../parse_util';
 
@@ -29,6 +31,8 @@ const _UNIT_TAG = 'unit';
 
 // http://docs.oasis-open.org/xliff/xliff-core/v2.0/os/xliff-core-v2.0-os.html
 export class Xliff2 extends Serializer {
+  constructor(private version: I18nVersion) { super(); }
+
   write(messages: i18n.Message[], locale: string|null): string {
     const visitor = new _WriteVisitor();
     const units: xml.Node[] = [];
@@ -85,7 +89,7 @@ export class Xliff2 extends Serializer {
   }
 
   load(content: string, url: string):
-      {locale: string, i18nNodesByMsgId: {[msgId: string]: i18n.Node[]}} {
+      {locale: string | null, i18nNodesByMsgId: {[msgId: string]: i18n.Node[]}} {
     // xliff to xml nodes
     const xliff2Parser = new Xliff2Parser();
     const {locale, msgIdToHtml, errors} = xliff2Parser.parse(content, url);
@@ -104,10 +108,15 @@ export class Xliff2 extends Serializer {
       throw new Error(`xliff2 parse errors:\n${errors.join('\n')}`);
     }
 
-    return {locale: locale !, i18nNodesByMsgId};
+    return {locale, i18nNodesByMsgId};
   }
 
-  digest(message: i18n.Message): string { return decimalDigest(message); }
+  digest(message: i18n.Message): string {
+    if (this.version === I18nVersion.V0) {
+      return decimalDigestDeprecated(message);
+    }
+    return decimalDigest(message);
+  }
 }
 
 class _WriteVisitor implements i18n.Visitor {