feat(ivy): updated translation const names (that include message ids) (#27185)

PR Close #27185

packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts

@@ -60,7 +60,7 @@ export class DecorationAnalyzer {
     new BaseDefDecoratorHandler(this.typeChecker, this.host),
     new ComponentDecoratorHandler(
         this.typeChecker, this.host, this.scopeRegistry, this.isCore, this.resourceLoader,
-        this.rootDirs, /* defaultPreserveWhitespaces */ false),
+        this.rootDirs, /* defaultPreserveWhitespaces */ false, /* i18nUseExternalIds */ true),
     new DirectiveDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, this.isCore),
     new InjectableDecoratorHandler(this.host, this.isCore),
     new NgModuleDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, this.isCore),

packages/compiler-cli/src/ngtsc/annotations/src/component.ts

@@ -40,7 +40,7 @@ export class ComponentDecoratorHandler implements
       private checker: ts.TypeChecker, private reflector: ReflectionHost,
       private scopeRegistry: SelectorScopeRegistry, private isCore: boolean,
       private resourceLoader: ResourceLoader, private rootDirs: string[],
-      private defaultPreserveWhitespaces: boolean) {}
+      private defaultPreserveWhitespaces: boolean, private i18nUseExternalIds: boolean) {}
 
   private literalCache = new Map<Decorator, ts.ObjectLiteralExpression>();
   private elementSchemaRegistry = new DomElementSchemaRegistry();
@@ -131,7 +131,7 @@ export class ComponentDecoratorHandler implements
     // Go through the root directories for this project, and select the one with the smallest
     // relative path representation.
     const filePath = node.getSourceFile().fileName;
-    const relativeFilePath = this.rootDirs.reduce<string|undefined>((previous, rootDir) => {
+    const relativeContextFilePath = this.rootDirs.reduce<string|undefined>((previous, rootDir) => {
       const candidate = path.posix.relative(rootDir, filePath);
       if (previous === undefined || candidate.length < previous.length) {
         return candidate;
@@ -142,7 +142,7 @@ export class ComponentDecoratorHandler implements
 
     const template = parseTemplate(
         templateStr, `${node.getSourceFile().fileName}#${node.name!.text}/template.html`,
-        {preserveWhitespaces}, relativeFilePath);
+        {preserveWhitespaces});
     if (template.errors !== undefined) {
       throw new Error(
           `Errors parsing template: ${template.errors.map(e => e.toString()).join(', ')}`);
@@ -212,7 +212,8 @@ export class ComponentDecoratorHandler implements
           directives: EMPTY_ARRAY,
           wrapDirectivesAndPipesInClosure: false,  //
           animations,
-          viewProviders
+          viewProviders,
+          i18nUseExternalIds: this.i18nUseExternalIds, relativeContextFilePath
         },
         metadataStmt: generateSetClassMetadataCall(node, this.reflector, this.isCore),
         parsedTemplate: template.nodes,

packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts

@@ -40,7 +40,7 @@ describe('ComponentDecoratorHandler', () => {
     const host = new TypeScriptReflectionHost(checker);
     const handler = new ComponentDecoratorHandler(
         checker, host, new SelectorScopeRegistry(checker, host), false, new NoopResourceLoader(),
-        [''], false);
+        [''], false, true);
     const TestCmp = getDeclaration(program, 'entry.ts', 'TestCmp', ts.isClassDeclaration);
     const detected = handler.detect(TestCmp, host.getDecoratorsOfDeclaration(TestCmp));
     if (detected === undefined) {

packages/compiler-cli/src/ngtsc/program.ts

@@ -220,7 +220,7 @@ export class NgtscProgram implements api.Program {
       new BaseDefDecoratorHandler(checker, this.reflector),
       new ComponentDecoratorHandler(
           checker, this.reflector, scopeRegistry, this.isCore, this.resourceLoader, this.rootDirs,
-          this.options.preserveWhitespaces || false),
+          this.options.preserveWhitespaces || false, this.options.i18nUseExternalIds !== false),
       new DirectiveDecoratorHandler(checker, this.reflector, scopeRegistry, this.isCore),
       new InjectableDecoratorHandler(this.reflector, this.isCore),
       new NgModuleDecoratorHandler(checker, this.reflector, scopeRegistry, this.isCore),

packages/compiler-cli/src/transformers/api.ts

@@ -151,6 +151,9 @@ export interface CompilerOptions extends ts.CompilerOptions {
   i18nInFile?: string;
   // How to handle missing messages
   i18nInMissingTranslations?: 'error'|'warning'|'ignore';
+  // Whether translation variable name should contain external message id
+  // (used by Closure Compiler's output of `goog.getMsg` for transition period)
+  i18nUseExternalIds?: boolean;
 
   // Whether to remove blank text nodes from compiled templates. It is `false` by default starting
   // from Angular 6.

packages/compiler-cli/src/transformers/program.ts

@@ -935,7 +935,8 @@ function getAotCompilerOptions(options: CompilerOptions): AotCompilerOptions {
 
   return {
     locale: options.i18nInLocale,
-    i18nFormat: options.i18nInFormat || options.i18nOutFormat, translations, missingTranslation,
+    i18nFormat: options.i18nInFormat || options.i18nOutFormat,
+    i18nUseExternalIds: options.i18nUseExternalIds, translations, missingTranslation,
     enableSummariesForJit: options.enableSummariesForJit,
     preserveWhitespaces: options.preserveWhitespaces,
     fullTemplateTypeCheck: options.fullTemplateTypeCheck,

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

@@ -553,6 +553,26 @@ describe('ngtsc behavioral tests', () => {
     expect(trim(jsContents)).toContain(trim(hostBindingsFn));
   });
 
+  it('should use proper default value for preserveWhitespaces config param', () => {
+    env.tsconfig();  // default is `false`
+    env.write(`test.ts`, `
+      import {Component} from '@angular/core';
+       @Component({
+        selector: 'test',
+        preserveWhitespaces: false,
+        template: \`
+          <div>
+            Template with whitespaces
+          </div>
+        \`
+      })
+      class FooCmp {}
+    `);
+    env.driveMain();
+    const jsContents = env.getContents('test.js');
+    expect(jsContents).toContain('text(1, " Template with whitespaces ");');
+  });
+
   it('should take preserveWhitespaces config option into account', () => {
     env.tsconfig({preserveWhitespaces: true});
     env.write(`test.ts`, `
@@ -593,6 +613,36 @@ describe('ngtsc behavioral tests', () => {
     expect(jsContents).toContain('text(1, " Template with whitespaces ");');
   });
 
+  it('should use proper default value for i18nUseExternalIds config param', () => {
+    env.tsconfig();  // default is `true`
+    env.write(`test.ts`, `
+      import {Component} from '@angular/core';
+       @Component({
+        selector: 'test',
+        template: '<div i18n>Some text</div>'
+      })
+      class FooCmp {}
+    `);
+    env.driveMain();
+    const jsContents = env.getContents('test.js');
+    expect(jsContents).toContain('i18n(1, MSG_EXTERNAL_8321000940098097247);');
+  });
+
+  it('should take i18nUseExternalIds config option into account', () => {
+    env.tsconfig({i18nUseExternalIds: false});
+    env.write(`test.ts`, `
+      import {Component} from '@angular/core';
+       @Component({
+        selector: 'test',
+        template: '<div i18n>Some text</div>'
+      })
+      class FooCmp {}
+    `);
+    env.driveMain();
+    const jsContents = env.getContents('test.js');
+    expect(jsContents).toContain('i18n(1, MSG_TEST_TS_0);');
+  });
+
   it('should correctly recognize local symbols', () => {
     env.tsconfig();
     env.write('module.ts', `

packages/compiler/src/aot/compiler_options.ts

@@ -11,6 +11,7 @@ import {MissingTranslationStrategy} from '../core';
 export interface AotCompilerOptions {
   locale?: string;
   i18nFormat?: string;
+  i18nUseExternalIds?: boolean;
   translations?: string;
   missingTranslation?: MissingTranslationStrategy;
   enableSummariesForJit?: boolean;

packages/compiler/src/jit_compiler_facade.ts

@@ -104,11 +104,9 @@ export class CompilerFacadeImpl implements CompilerFacade {
     const constantPool = new ConstantPool();
 
     // Parse the template and check for errors.
-    const template = parseTemplate(
+    const template = parseTemplate(facade.template, sourceMapUrl, {
-        facade.template, sourceMapUrl, {
+      preserveWhitespaces: facade.preserveWhitespaces || false,
-          preserveWhitespaces: facade.preserveWhitespaces || false,
+    });
-        },
-        '');
     if (template.errors !== undefined) {
       const errors = template.errors.map(err => err.toString()).join(', ');
       throw new Error(`Errors during JIT compilation of template for ${facade.name}: ${errors}`);
@@ -129,6 +127,8 @@ export class CompilerFacadeImpl implements CompilerFacade {
           animations: facade.animations != null ? new WrappedNodeExpr(facade.animations) : null,
           viewProviders: facade.viewProviders != null ? new WrappedNodeExpr(facade.viewProviders) :
                                                         null,
+          relativeContextFilePath: '',
+          i18nUseExternalIds: true,
         },
         constantPool, makeBindingParser());
     const preStatements = [...constantPool.statements, ...res.statements];

packages/compiler/src/render3/view/api.ts

@@ -132,13 +132,6 @@ export interface R3ComponentMetadata extends R3DirectiveMetadata {
      * Selectors found in the <ng-content> tags in the template.
      */
     ngContentSelectors: string[];
-
-    /**
-     * Path to the .ts file in which this template's generated code will be included, relative to
-     * the compilation root. This will be used to generate identifiers that need to be globally
-     * unique in certain contexts (such as g3).
-     */
-    relativeContextFilePath: string;
   };
 
   /**
@@ -190,6 +183,20 @@ export interface R3ComponentMetadata extends R3DirectiveMetadata {
    * The list of view providers defined in the component.
    */
   viewProviders: o.Expression|null;
+
+
+  /**
+   * Path to the .ts file in which this template's generated code will be included, relative to
+   * the compilation root. This will be used to generate identifiers that need to be globally
+   * unique in certain contexts (such as g3).
+   */
+  relativeContextFilePath: string;
+
+  /**
+   * Whether translation variable name should contain external message id
+   * (used by Closure Compiler's output of `goog.getMsg` for transition period)
+   */
+  i18nUseExternalIds: boolean;
 }
 
 /**

packages/compiler/src/render3/view/compiler.ts

@@ -254,7 +254,7 @@ export function compileComponentFromMetadata(
   const templateBuilder = new TemplateDefinitionBuilder(
       constantPool, BindingScope.ROOT_SCOPE, 0, templateTypeName, null, null, templateName,
       meta.viewQueries, directiveMatcher, directivesUsed, meta.pipes, pipesUsed, R3.namespaceHTML,
-      meta.template.relativeContextFilePath);
+      meta.relativeContextFilePath, meta.i18nUseExternalIds);
 
   const templateFunctionExpression = templateBuilder.buildTemplateFunction(
       template.nodes, [], template.hasNgContent, template.ngContentSelectors);
@@ -373,7 +373,6 @@ export function compileComponentFromRender2(
       nodes: render3Ast.nodes,
       hasNgContent: render3Ast.hasNgContent,
       ngContentSelectors: render3Ast.ngContentSelectors,
-      relativeContextFilePath: '',
     },
     directives: [],
     pipes: typeMapToExpressionMap(pipeTypeByName, outputCtx),
@@ -384,7 +383,9 @@ export function compileComponentFromRender2(
         (summary.template && summary.template.encapsulation) || core.ViewEncapsulation.Emulated,
     animations: null,
     viewProviders:
-        component.viewProviders.length > 0 ? new o.WrappedNodeExpr(component.viewProviders) : null
+        component.viewProviders.length > 0 ? new o.WrappedNodeExpr(component.viewProviders) : null,
+    relativeContextFilePath: '',
+    i18nUseExternalIds: true,
   };
   const res = compileComponentFromMetadata(meta, outputCtx.constantPool, bindingParser);
 

packages/compiler/src/render3/view/i18n/util.ts

@@ -19,12 +19,6 @@ const TRANSLATION_PREFIX = 'MSG_';
 /** Closure uses `goog.getMsg(message)` to lookup translations */
 const GOOG_GET_MSG = 'goog.getMsg';
 
-/** String key that is used to provide backup id of translatable message in Closure */
-const BACKUP_MESSAGE_ID = 'BACKUP_MESSAGE_ID';
-
-/** Regexp to identify whether backup id already provided in description */
-const BACKUP_MESSAGE_ID_REGEXP = new RegExp(BACKUP_MESSAGE_ID);
-
 /** I18n separators for metadata **/
 const I18N_MEANING_SEPARATOR = '|';
 const I18N_ID_SEPARATOR = '@@';
@@ -63,15 +57,11 @@ function i18nTranslationToDeclStmt(
 // to a JsDoc statement formatted as expected by the Closure compiler.
 function i18nMetaToDocStmt(meta: I18nMeta): o.JSDocCommentStmt|null {
   const tags: o.JSDocTag[] = [];
-  const {id, description, meaning} = meta;
+  if (meta.description) {
-  if (id || description) {
+    tags.push({tagName: o.JSDocTagName.Desc, text: meta.description});
-    const hasBackupId = !!description && BACKUP_MESSAGE_ID_REGEXP.test(description);
-    const text =
-        id && !hasBackupId ? `[${BACKUP_MESSAGE_ID}:${id}] ${description || ''}` : description;
-    tags.push({tagName: o.JSDocTagName.Desc, text: text !.trim()});
   }
-  if (meaning) {
+  if (meta.meaning) {
-    tags.push({tagName: o.JSDocTagName.Meaning, text: meaning});
+    tags.push({tagName: o.JSDocTagName.Meaning, text: meta.meaning});
   }
   return tags.length == 0 ? null : new o.JSDocCommentStmt(tags);
 }
@@ -92,9 +82,9 @@ export function hasI18nAttrs(element: html.Element): boolean {
   return element.attrs.some((attr: html.Attribute) => isI18nAttribute(attr.name));
 }
 
-export function metaFromI18nMessage(message: i18n.Message): I18nMeta {
+export function metaFromI18nMessage(message: i18n.Message, id: string | null = null): I18nMeta {
   return {
-    id: message.id || '',
+    id: typeof id === 'string' ? id : message.id || '',
     meaning: message.meaning || '',
     description: message.description || ''
   };
@@ -222,8 +212,14 @@ export function formatI18nPlaceholderName(name: string): string {
   return postfix ? `${raw}_${postfix}` : raw;
 }
 
-export function getTranslationConstPrefix(fileBasedSuffix: string): string {
+/**
-  return `${TRANSLATION_PREFIX}${fileBasedSuffix}`.toUpperCase();
+ * Generates a prefix for translation const name.
+ *
+ * @param extra Additional local prefix that should be injected into translation var name
+ * @returns Complete translation const prefix
+ */
+export function getTranslationConstPrefix(extra: string): string {
+  return `${TRANSLATION_PREFIX}${extra}`.toUpperCase();
 }
 
 /**
@@ -246,7 +242,7 @@ export function getTranslationDeclStmts(
     statements.push(docStatements);
   }
   if (transformFn) {
-    const raw = o.variable(`${variable.name}_RAW`);
+    const raw = o.variable(`${variable.name}$$RAW`);
     statements.push(i18nTranslationToDeclStmt(raw, message, params));
     statements.push(
         variable.set(transformFn(raw)).toDeclStmt(o.INFERRED_TYPE, [o.StmtModifier.Final]));

packages/compiler/src/render3/view/template.ts

@@ -109,7 +109,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
       private viewQueries: R3QueryMetadata[], private directiveMatcher: SelectorMatcher|null,
       private directives: Set<o.Expression>, private pipeTypeByName: Map<string, o.Expression>,
       private pipes: Set<o.Expression>, private _namespace: o.ExternalReference,
-      private relativeContextFilePath: string) {
+      private relativeContextFilePath: string, private i18nUseExternalIds: boolean) {
     // view queries can take up space in data and allocation happens earlier (in the "viewQuery"
     // function)
     this._dataIndex = viewQueries.length;
@@ -184,8 +184,9 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
     // - this template has parent i18n context
     // - or the template has i18n meta associated with it,
     //   but it's not initiated by the Element (e.g. <ng-template i18n>)
-    const initI18nContext = this.i18nContext ||
+    const initI18nContext =
-        (isI18nRootNode(i18n) && !(isSingleElementTemplate(nodes) && nodes[0].i18n === i18n));
+        this.i18nContext || (isI18nRootNode(i18n) && !isSingleI18nIcu(i18n) &&
+                             !(isSingleElementTemplate(nodes) && nodes[0].i18n === i18n));
     const selfClosingI18nInstruction = hasTextChildrenOnly(nodes);
     if (initI18nContext) {
       this.i18nStart(null, i18n !, selfClosingI18nInstruction);
@@ -254,8 +255,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
 
   i18nTranslate(
       message: i18n.Message, params: {[name: string]: o.Expression} = {}, ref?: o.ReadVarExpr,
-      transformFn?: (raw: o.ReadVarExpr) => o.Expression): o.Expression {
+      transformFn?: (raw: o.ReadVarExpr) => o.Expression): o.ReadVarExpr {
-    const _ref = ref || this.i18nAllocateRef();
+    const _ref = ref || this.i18nAllocateRef(message.id);
     const _params: {[key: string]: any} = {};
     if (params && Object.keys(params).length) {
       Object.keys(params).forEach(key => _params[formatI18nPlaceholderName(key)] = params[key]);
@@ -296,9 +297,16 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
     return bound;
   }
 
-  i18nAllocateRef() {
+  i18nAllocateRef(messageId: string): o.ReadVarExpr {
-    const prefix = getTranslationConstPrefix(this.fileBasedI18nSuffix);
+    let name: string;
-    return o.variable(this.constantPool.uniqueName(prefix));
+    if (this.i18nUseExternalIds) {
+      const prefix = getTranslationConstPrefix(`EXTERNAL_`);
+      name = `${prefix}${messageId}`;
+    } else {
+      const prefix = getTranslationConstPrefix(this.fileBasedI18nSuffix);
+      name = this.constantPool.uniqueName(prefix);
+    }
+    return o.variable(name);
   }
 
   i18nUpdateRef(context: I18nContext): void {
@@ -350,7 +358,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
     if (this.i18nContext) {
       this.i18n = this.i18nContext.forkChildContext(index, this.templateIndex !, meta);
     } else {
-      const ref = this.i18nAllocateRef();
+      const ref = this.i18nAllocateRef((meta as i18n.Message).id);
       this.i18n = new I18nContext(index, ref, 0, this.templateIndex, meta);
     }
 
@@ -434,7 +442,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
     const stylingBuilder = new StylingBuilder(o.literal(elementIndex), null);
 
     let isNonBindableMode: boolean = false;
-    const isI18nRootElement: boolean = isI18nRootNode(element.i18n);
+    const isI18nRootElement: boolean =
+        isI18nRootNode(element.i18n) && !isSingleI18nIcu(element.i18n);
 
     if (isI18nRootElement && this.i18n) {
       throw new Error(`Could not mark an element as translatable inside of a translatable section`);
@@ -704,7 +713,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
     const templateVisitor = new TemplateDefinitionBuilder(
         this.constantPool, this._bindingScope, this.level + 1, contextName, this.i18n,
         templateIndex, templateName, [], this.directiveMatcher, this.directives,
-        this.pipeTypeByName, this.pipes, this._namespace, this.fileBasedI18nSuffix);
+        this.pipeTypeByName, this.pipes, this._namespace, this.fileBasedI18nSuffix,
+        this.i18nUseExternalIds);
 
     // Nested templates must not be visited until after their parent templates have completed
     // processing, so they are queued here until after the initial pass. Otherwise, we wouldn't
@@ -1383,25 +1393,14 @@ function interpolate(args: o.Expression[]): o.Expression {
  * @param templateUrl URL to use for source mapping of the parsed template
  */
 export function parseTemplate(
-    template: string, templateUrl: string, options: {preserveWhitespaces?: boolean} = {},
+    template: string, templateUrl: string, options: {preserveWhitespaces?: boolean}):
-    relativeContextFilePath: string): {
+    {errors?: ParseError[], nodes: t.Node[], hasNgContent: boolean, ngContentSelectors: string[]} {
-  errors?: ParseError[],
-  nodes: t.Node[],
-  hasNgContent: boolean,
-  ngContentSelectors: string[],
-  relativeContextFilePath: string
-} {
   const bindingParser = makeBindingParser();
   const htmlParser = new HtmlParser();
   const parseResult = htmlParser.parse(template, templateUrl, true);
 
   if (parseResult.errors && parseResult.errors.length > 0) {
-    return {
+    return {errors: parseResult.errors, nodes: [], hasNgContent: false, ngContentSelectors: []};
-      errors: parseResult.errors,
-      nodes: [],
-      hasNgContent: false,
-      ngContentSelectors: [], relativeContextFilePath
-    };
   }
 
   let rootNodes: html.Node[] = parseResult.rootNodes;
@@ -1426,15 +1425,10 @@ export function parseTemplate(
   const {nodes, hasNgContent, ngContentSelectors, errors} =
       htmlAstToRender3Ast(rootNodes, bindingParser);
   if (errors && errors.length > 0) {
-    return {
+    return {errors, nodes: [], hasNgContent: false, ngContentSelectors: []};
-      errors,
-      nodes: [],
-      hasNgContent: false,
-      ngContentSelectors: [], relativeContextFilePath
-    };
   }
 
-  return {nodes, hasNgContent, ngContentSelectors, relativeContextFilePath};
+  return {nodes, hasNgContent, ngContentSelectors};
 }
 
 /**
@@ -1478,4 +1472,4 @@ function hasTextChildrenOnly(children: t.Node[]): boolean {
   return !children.find(
       child =>
           !(child instanceof t.Text || child instanceof t.BoundText || child instanceof t.Icu));
-}
+}

packages/compiler/test/render3/view/binding_spec.ts

@@ -29,8 +29,7 @@ function makeSelectorMatcher(): SelectorMatcher<DirectiveMeta> {
 
 describe('t2 binding', () => {
   it('should bind a simple template', () => {
-    const template =
+    const template = parseTemplate('<div *ngFor="let item of items">{{item.name}}</div>', '', {});
-        parseTemplate('<div *ngFor="let item of items">{{item.name}}</div>', '', {}, '');
     const binder = new R3TargetBinder(new SelectorMatcher<DirectiveMeta>());
     const res = binder.bind({template: template.nodes});
 
@@ -48,8 +47,7 @@ describe('t2 binding', () => {
   });
 
   it('should match directives when binding a simple template', () => {
-    const template =
+    const template = parseTemplate('<div *ngFor="let item of items">{{item.name}}</div>', '', {});
-        parseTemplate('<div *ngFor="let item of items">{{item.name}}</div>', '', {}, '');
     const binder = new R3TargetBinder(makeSelectorMatcher());
     const res = binder.bind({template: template.nodes});
     const tmpl = template.nodes[0] as a.Template;