From f1a063298e9ab4b30536e877e2d6ecc34873846f Mon Sep 17 00:00:00 2001
From: Oussama Ben Brahim <benbraou@gmail.com>
Date: Tue, 6 Feb 2018 00:37:05 +0100
Subject: [PATCH] feat(core): set preserveWhitespaces to false by default
 (#22046)

Fixes #22027

PR Close #22046
---
 aio/content/guide/aot-compiler.md                    |  6 ++----
 packages/bazel/src/ng_module.bzl                     |  3 +--
 packages/compiler-cli/src/transformers/api.ts        |  4 ++--
 packages/compiler/src/config.ts                      |  2 +-
 packages/compiler/test/config_spec.ts                | 11 ++++++++++-
 packages/core/src/metadata/directives.ts             | 12 ++++++------
 packages/core/test/debug/debug_node_spec.ts          |  4 +---
 packages/core/test/linker/integration_spec.ts        |  4 ++--
 .../ts/e2e_test/ngComponentOutlet_spec.ts            |  2 +-
 .../examples/common/ngIf/ts/e2e_test/ngIf_spec.ts    |  6 +++---
 .../test/testing_public_browser_spec.ts              |  2 +-
 .../platform-browser/test/testing_public_spec.ts     |  2 +-
 packages/router/test/integration.spec.ts             |  6 +++---
 13 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/aio/content/guide/aot-compiler.md b/aio/content/guide/aot-compiler.md
index 4ee81ab438..c55dc24fd4 100644
--- a/aio/content/guide/aot-compiler.md
+++ b/aio/content/guide/aot-compiler.md
@@ -92,7 +92,7 @@ You can control your app compilation by providing template compiler options in t
   },
   "angularCompilerOptions": {
     "fullTemplateTypeCheck": true,
-    "preserveWhitespaces": false,
+    "preserveWhitespaces": true,
     ...
   }
 }
@@ -234,9 +234,7 @@ done manually.
 ### *preserveWhitespaces*
 
 This option tells the compiler whether to remove blank text nodes from compiled templates.
-This option is `true` by default.
-
-*Note*: It is recommended to set this explicitly to `false` as it emits smaller template factory modules and might be set to `false` by default in the future.
+As of v6, this option is `false` by default, which results in smaller emitted template factory modules.
 
 ### *allowEmptyCodegenFiles*
 
diff --git a/packages/bazel/src/ng_module.bzl b/packages/bazel/src/ng_module.bzl
index 7f2c33b327..5041dccc49 100644
--- a/packages/bazel/src/ng_module.bzl
+++ b/packages/bazel/src/ng_module.bzl
@@ -91,8 +91,7 @@ def _ngc_tsconfig(ctx, files, srcs, **kwargs):
           "enableSummariesForJit": True,
           "fullTemplateTypeCheck": ctx.attr.type_check,
           # FIXME: wrong place to de-dupe
-          "expectedOut": depset([o.path for o in expected_outs]).to_list(),
-          "preserveWhitespaces": False,
+          "expectedOut": depset([o.path for o in expected_outs]).to_list()
       }
   })
 
diff --git a/packages/compiler-cli/src/transformers/api.ts b/packages/compiler-cli/src/transformers/api.ts
index 6679592f0d..b428f8fe0f 100644
--- a/packages/compiler-cli/src/transformers/api.ts
+++ b/packages/compiler-cli/src/transformers/api.ts
@@ -148,8 +148,8 @@ export interface CompilerOptions extends ts.CompilerOptions {
   // How to handle missing messages
   i18nInMissingTranslations?: 'error'|'warning'|'ignore';
 
-  // Whether to remove blank text nodes from compiled templates. It is `true` by default
-  // in Angular 5 and will be re-visited in Angular 6.
+  // Whether to remove blank text nodes from compiled templates. It is `false` by default starting
+  // from Angular 6.
   preserveWhitespaces?: boolean;
 
   /** generate all possible generated files  */
diff --git a/packages/compiler/src/config.ts b/packages/compiler/src/config.ts
index 0826e3c1ab..eb0e7cddc7 100644
--- a/packages/compiler/src/config.ts
+++ b/packages/compiler/src/config.ts
@@ -46,6 +46,6 @@ export class CompilerConfig {
 }
 
 export function preserveWhitespacesDefault(
-    preserveWhitespacesOption: boolean | null, defaultSetting = true): boolean {
+    preserveWhitespacesOption: boolean | null, defaultSetting = false): boolean {
   return preserveWhitespacesOption === null ? defaultSetting : preserveWhitespacesOption;
 }
diff --git a/packages/compiler/test/config_spec.ts b/packages/compiler/test/config_spec.ts
index 6d164afc63..459e80cf60 100644
--- a/packages/compiler/test/config_spec.ts
+++ b/packages/compiler/test/config_spec.ts
@@ -7,7 +7,7 @@
  */
 
 import {MissingTranslationStrategy} from '@angular/core';
-import {CompilerConfig} from '../src/config';
+import {CompilerConfig, preserveWhitespacesDefault} from '../src/config';
 
 {
   describe('compiler config', () => {
@@ -16,4 +16,13 @@ import {CompilerConfig} from '../src/config';
       expect(config.missingTranslation).toEqual(MissingTranslationStrategy.Error);
     });
   });
+
+  describe('preserveWhitespacesDefault', () => {
+    it('should return the default `false` setting when no preserveWhitespacesOption are provided',
+       () => { expect(preserveWhitespacesDefault(null)).toEqual(false); });
+    it('should return the preserveWhitespacesOption when provided as a parameter', () => {
+      expect(preserveWhitespacesDefault(true)).toEqual(true);
+      expect(preserveWhitespacesDefault(false)).toEqual(false);
+    });
+  });
 }
diff --git a/packages/core/src/metadata/directives.ts b/packages/core/src/metadata/directives.ts
index 25571fcb54..d503ad72be 100644
--- a/packages/core/src/metadata/directives.ts
+++ b/packages/core/src/metadata/directives.ts
@@ -693,13 +693,13 @@ export interface Component extends Directive {
    * - text nodes are left as-is inside HTML tags where whitespaces are significant (ex. `<pre>`,
    *   `<textarea>`).
    *
-   * Described transformations can (potentially) influence DOM nodes layout so the
-   * `preserveWhitespaces` option is `true` be default (no whitespace removal).
-   * In Angular 5 you need to opt-in for whitespace removal (but we might revisit the default
-   * setting in Angular 6 or later). If you want to change the default setting for all components
-   * in your application you can use the `preserveWhitespaces` option of the AOT compiler.
+   * Described transformations may (potentially) influence DOM nodes layout. However, the impact
+   * should so be minimal. That's why starting from Angular 6, the
+   * `preserveWhitespaces` option is `false` by default (whitespace removal).
+   * If you want to change the default setting for all components in your application you can use
+   * the `preserveWhitespaces` option of the AOT compiler.
    *
-   * Even if you decide to opt-in for whitespace removal there are ways of preserving whitespaces
+   * Even with the default behavior of whitespace removal, there are ways of preserving whitespaces
    * in certain fragments of a template. You can either exclude entire DOM sub-tree by using the
    * `ngPreserveWhitespaces` attribute, ex.:
    *
diff --git a/packages/core/test/debug/debug_node_spec.ts b/packages/core/test/debug/debug_node_spec.ts
index 97d365d9fe..bb73be6b3a 100644
--- a/packages/core/test/debug/debug_node_spec.ts
+++ b/packages/core/test/debug/debug_node_spec.ts
@@ -191,9 +191,7 @@ class TestApp {
     it('should list all child nodes', () => {
       fixture = TestBed.createComponent(ParentComp);
       fixture.detectChanges();
-
-      // The root component has 3 elements and 2 text node children.
-      expect(fixture.debugElement.childNodes.length).toEqual(5);
+      expect(fixture.debugElement.childNodes.length).toEqual(3);
     });
 
     it('should list all component child elements', () => {
diff --git a/packages/core/test/linker/integration_spec.ts b/packages/core/test/linker/integration_spec.ts
index 9270c2891c..aa20340059 100644
--- a/packages/core/test/linker/integration_spec.ts
+++ b/packages/core/test/linker/integration_spec.ts
@@ -1280,7 +1280,7 @@ function declareTests({useJit}: {useJit: boolean}) {
         fixture.detectChanges();
         expect(fixture.nativeElement)
             .toHaveText(
-                'Default Interpolation\nCustom Interpolation A\nCustom Interpolation B (Default Interpolation)');
+                'Default InterpolationCustom Interpolation ACustom Interpolation B (Default Interpolation)');
       });
     });
 
@@ -1792,7 +1792,7 @@ function declareTests({useJit}: {useJit: boolean}) {
            const f = TestBed.configureTestingModule({declarations: [MyCmp]}).createComponent(MyCmp);
            f.detectChanges();
 
-           expect(f.nativeElement.childNodes.length).toBe(3);
+           expect(f.nativeElement.childNodes.length).toBe(2);
          }));
 
       it('should not remove whitespaces when explicitly requested not to do so', async(() => {
diff --git a/packages/examples/common/ngComponentOutlet/ts/e2e_test/ngComponentOutlet_spec.ts b/packages/examples/common/ngComponentOutlet/ts/e2e_test/ngComponentOutlet_spec.ts
index f205f775e3..fd6110a4b5 100644
--- a/packages/examples/common/ngComponentOutlet/ts/e2e_test/ngComponentOutlet_spec.ts
+++ b/packages/examples/common/ngComponentOutlet/ts/e2e_test/ngComponentOutlet_spec.ts
@@ -29,7 +29,7 @@ describe('ngComponentOutlet', () => {
     it('should render complete', () => {
       browser.get(URL);
       waitForElement('ng-component-outlet-complete-example');
-      expect(element.all(by.css('complete-component')).getText()).toEqual(['Complete: Ahoj Svet!']);
+      expect(element.all(by.css('complete-component')).getText()).toEqual(['Complete: AhojSvet!']);
     });
 
     it('should render other module', () => {
diff --git a/packages/examples/common/ngIf/ts/e2e_test/ngIf_spec.ts b/packages/examples/common/ngIf/ts/e2e_test/ngIf_spec.ts
index 57a5e22f40..b0ba5c41c6 100644
--- a/packages/examples/common/ngIf/ts/e2e_test/ngIf_spec.ts
+++ b/packages/examples/common/ngIf/ts/e2e_test/ngIf_spec.ts
@@ -48,13 +48,13 @@ describe('ngIf', () => {
       browser.get(URL);
       waitForElement(comp);
       expect(element.all(by.css(comp)).get(0).getText())
-          .toEqual('hide Switch Primary show = true\nPrimary text to show');
+          .toEqual('hideSwitch Primary show = true\nPrimary text to show');
       element.all(by.css(comp + ' button')).get(1).click();
       expect(element.all(by.css(comp)).get(0).getText())
-          .toEqual('hide Switch Primary show = true\nSecondary text to show');
+          .toEqual('hideSwitch Primary show = true\nSecondary text to show');
       element.all(by.css(comp + ' button')).get(0).click();
       expect(element.all(by.css(comp)).get(0).getText())
-          .toEqual('show Switch Primary show = false\nAlternate text while primary text is hidden');
+          .toEqual('showSwitch Primary show = false\nAlternate text while primary text is hidden');
     });
   });
 
diff --git a/packages/platform-browser-dynamic/test/testing_public_browser_spec.ts b/packages/platform-browser-dynamic/test/testing_public_browser_spec.ts
index a009dcfa02..82c071e761 100644
--- a/packages/platform-browser-dynamic/test/testing_public_browser_spec.ts
+++ b/packages/platform-browser-dynamic/test/testing_public_browser_spec.ts
@@ -123,7 +123,7 @@ class BadTemplateUrl {
            TestBed.compileComponents().then(() => {
              const componentFixture = TestBed.createComponent(ExternalTemplateComp);
              componentFixture.detectChanges();
-             expect(componentFixture.nativeElement.textContent).toEqual('from external template\n');
+             expect(componentFixture.nativeElement.textContent).toEqual('from external template');
            });
          }),
          10000);  // Long timeout here because this test makes an actual ResourceLoader request, and
diff --git a/packages/platform-browser/test/testing_public_spec.ts b/packages/platform-browser/test/testing_public_spec.ts
index 8ca4b05287..f20b78a0c7 100644
--- a/packages/platform-browser/test/testing_public_spec.ts
+++ b/packages/platform-browser/test/testing_public_spec.ts
@@ -307,7 +307,7 @@ class CompWithUrlTemplate {
         it('should allow to createSync components with templateUrl after explicit async compilation',
            () => {
              const fixture = TestBed.createComponent(CompWithUrlTemplate);
-             expect(fixture.nativeElement).toHaveText('from external template\n');
+             expect(fixture.nativeElement).toHaveText('from external template');
            });
       });
 
diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts
index 9a09b4ba26..2848c4b30c 100644
--- a/packages/router/test/integration.spec.ts
+++ b/packages/router/test/integration.spec.ts
@@ -1618,20 +1618,20 @@ describe('Integration', () => {
 
          router.navigateByUrl('/');
          advance(fixture);
-         expect(fixture.nativeElement).toHaveText(' ');
+         expect(fixture.nativeElement).toHaveText('');
          const cmp = fixture.componentInstance;
 
          cmp.show = true;
          advance(fixture);
 
-         expect(fixture.nativeElement).toHaveText('link ');
+         expect(fixture.nativeElement).toHaveText('link');
          const native = fixture.nativeElement.querySelector('a');
 
          expect(native.getAttribute('href')).toEqual('/simple');
          native.click();
          advance(fixture);
 
-         expect(fixture.nativeElement).toHaveText('link simple');
+         expect(fixture.nativeElement).toHaveText('linksimple');
        })));
 
     it('should support query params and fragments',