From 66a5dab85a84c8173c17ab90a3572cf0351f111b Mon Sep 17 00:00:00 2001
From: Olivier Combe <olivier.combe@gmail.com>
Date: Fri, 1 Sep 2017 14:50:58 +0200
Subject: [PATCH] feat(compiler): deprecate i18n comments in favor of
 `ng-container` (#18998)

PR Close #18998
---
 .../examples/i18n/src/app/app.component.html     |  8 --------
 aio/content/guide/i18n.md                        | 16 ++++------------
 packages/compiler/src/i18n/extractor_merger.ts   | 10 +++++++++-
 .../compiler/test/i18n/extractor_merger_spec.ts  |  5 +++++
 4 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/aio/content/examples/i18n/src/app/app.component.html b/aio/content/examples/i18n/src/app/app.component.html
index 36d0188982..ec46ed8095 100644
--- a/aio/content/examples/i18n/src/app/app.component.html
+++ b/aio/content/examples/i18n/src/app/app.component.html
@@ -11,14 +11,6 @@
 
 <br />
 
-<!--#docregion i18n-with-comment-->
-<!--i18n: optional meaning|optional description -->
-I don't output any element either
-<!--/i18n-->
-<!--#enddocregion i18n-with-comment-->
-
-<br />
-
 <!--#docregion i18n-title-translate-->
 <img [src]="logo" i18n-title title="Angular logo" />
 <!--#enddocregion i18n-title-translate-->
diff --git a/aio/content/guide/i18n.md b/aio/content/guide/i18n.md
index d12f9dfbec..fe997fa129 100644
--- a/aio/content/guide/i18n.md
+++ b/aio/content/guide/i18n.md
@@ -178,22 +178,14 @@ Here is a _meaning_ and a _description_ and the _id_ at the end:
 
 ### Translate text without creating an element
 
-Suppose there is a stretch of text that you'd like to translate.
-You could wrap it in a `<span>` tag but for some reason (CSS comes to mind)
-you don't want to create a new DOM element merely to facilitate translation.
-
-Here are two techniques to try.
-
-(1) Wrap the text in an `<ng-container>` element. The `<ng-container>` is never rendered:
+If there is a stretch of text that you'd like to translate, you could wrap it in a `<span>` tag.
+But if you don't want to create a new DOM element merely to facilitate translation,
+you can wrap the text in an `<ng-container>` element.
+The `<ng-container>` will be transformed into an html comment:
 
 <code-example path="i18n/src/app/app.component.html" region="i18n-ng-container" title="src/app/app.component.html" linenums="false">
 </code-example>
 
-(2) Wrap the text in a pair of HTML comments:
-
-<code-example path="i18n/src/app/app.component.html" region="i18n-with-comment" title="src/app/app.component.html" linenums="false">
-</code-example>
-
 {@a translate-attributes}
 
 ## Add _i18n_ translation attributes
diff --git a/packages/compiler/src/i18n/extractor_merger.ts b/packages/compiler/src/i18n/extractor_merger.ts
index 1cae8b9464..4cda98b0b6 100644
--- a/packages/compiler/src/i18n/extractor_merger.ts
+++ b/packages/compiler/src/i18n/extractor_merger.ts
@@ -9,7 +9,6 @@
 import * as html from '../ml_parser/ast';
 import {InterpolationConfig} from '../ml_parser/interpolation_config';
 import {ParseTreeResult} from '../ml_parser/parser';
-
 import * as i18n from './i18n_ast';
 import {createI18nMessageFactory} from './i18n_parser';
 import {I18nError} from './parse_util';
@@ -20,6 +19,7 @@ const _I18N_ATTR_PREFIX = 'i18n-';
 const _I18N_COMMENT_PREFIX_REGEXP = /^i18n:?/;
 const MEANING_SEPARATOR = '|';
 const ID_SEPARATOR = '@@';
+let i18nCommentsWarned = false;
 
 /**
  * Extract translatable messages from an html AST
@@ -176,6 +176,14 @@ class _Visitor implements html.Visitor {
     if (!this._inI18nNode && !this._inIcu) {
       if (!this._inI18nBlock) {
         if (isOpening) {
+          // deprecated from v5 you should use <ng-container i18n> instead of i18n comments
+          if (!i18nCommentsWarned && <any>console && <any>console.warn) {
+            i18nCommentsWarned = true;
+            const details = comment.sourceSpan.details ? `, ${comment.sourceSpan.details}` : '';
+            // TODO(ocombe): use a log service once there is a public one available
+            console.warn(
+                ` I18n comments are deprecated, use an <ng - container> element instead (${comment.sourceSpan.start}${details})`);
+          }
           this._inI18nBlock = true;
           this._blockStartDepth = this._depth;
           this._blockChildren = [];
diff --git a/packages/compiler/test/i18n/extractor_merger_spec.ts b/packages/compiler/test/i18n/extractor_merger_spec.ts
index 1cca886b3e..ff1809672a 100644
--- a/packages/compiler/test/i18n/extractor_merger_spec.ts
+++ b/packages/compiler/test/i18n/extractor_merger_spec.ts
@@ -405,6 +405,11 @@ export function main() {
     });
 
     describe('blocks', () => {
+      it('should console.warn if we use i18n comments', () => {
+        // TODO(ocombe): expect a warning message when we have a proper log service
+        extract('<!-- i18n --><p><b i18n-title="m|d" title="msg"></b></p><!-- /i18n -->');
+      });
+
       it('should merge blocks', () => {
         const HTML = `before<!-- i18n --><p>foo</p><span><i>bar</i></span><!-- /i18n -->after`;
         expect(fakeTranslate(HTML))