feat(core): set preserveWhitespaces to false by default (#22046)

Fixes #22027

PR Close #22046
This commit is contained in:
Oussama Ben Brahim 2018-02-06 00:37:05 +01:00 committed by Victor Berchet
parent d241532488
commit f1a063298e
13 changed files with 34 additions and 30 deletions

View File

@ -92,7 +92,7 @@ You can control your app compilation by providing template compiler options in t
}, },
"angularCompilerOptions": { "angularCompilerOptions": {
"fullTemplateTypeCheck": true, "fullTemplateTypeCheck": true,
"preserveWhitespaces": false, "preserveWhitespaces": true,
... ...
} }
} }
@ -234,9 +234,7 @@ done manually.
### *preserveWhitespaces* ### *preserveWhitespaces*
This option tells the compiler whether to remove blank text nodes from compiled templates. This option tells the compiler whether to remove blank text nodes from compiled templates.
This option is `true` by default. As of v6, this option is `false` by default, which results in smaller emitted template factory modules.
*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.
### *allowEmptyCodegenFiles* ### *allowEmptyCodegenFiles*

View File

@ -91,8 +91,7 @@ def _ngc_tsconfig(ctx, files, srcs, **kwargs):
"enableSummariesForJit": True, "enableSummariesForJit": True,
"fullTemplateTypeCheck": ctx.attr.type_check, "fullTemplateTypeCheck": ctx.attr.type_check,
# FIXME: wrong place to de-dupe # FIXME: wrong place to de-dupe
"expectedOut": depset([o.path for o in expected_outs]).to_list(), "expectedOut": depset([o.path for o in expected_outs]).to_list()
"preserveWhitespaces": False,
} }
}) })

View File

@ -148,8 +148,8 @@ export interface CompilerOptions extends ts.CompilerOptions {
// How to handle missing messages // How to handle missing messages
i18nInMissingTranslations?: 'error'|'warning'|'ignore'; i18nInMissingTranslations?: 'error'|'warning'|'ignore';
// Whether to remove blank text nodes from compiled templates. It is `true` by default // Whether to remove blank text nodes from compiled templates. It is `false` by default starting
// in Angular 5 and will be re-visited in Angular 6. // from Angular 6.
preserveWhitespaces?: boolean; preserveWhitespaces?: boolean;
/** generate all possible generated files */ /** generate all possible generated files */

View File

@ -46,6 +46,6 @@ export class CompilerConfig {
} }
export function preserveWhitespacesDefault( export function preserveWhitespacesDefault(
preserveWhitespacesOption: boolean | null, defaultSetting = true): boolean { preserveWhitespacesOption: boolean | null, defaultSetting = false): boolean {
return preserveWhitespacesOption === null ? defaultSetting : preserveWhitespacesOption; return preserveWhitespacesOption === null ? defaultSetting : preserveWhitespacesOption;
} }

View File

@ -7,7 +7,7 @@
*/ */
import {MissingTranslationStrategy} from '@angular/core'; import {MissingTranslationStrategy} from '@angular/core';
import {CompilerConfig} from '../src/config'; import {CompilerConfig, preserveWhitespacesDefault} from '../src/config';
{ {
describe('compiler config', () => { describe('compiler config', () => {
@ -16,4 +16,13 @@ import {CompilerConfig} from '../src/config';
expect(config.missingTranslation).toEqual(MissingTranslationStrategy.Error); 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);
});
});
} }

View File

@ -693,13 +693,13 @@ export interface Component extends Directive {
* - text nodes are left as-is inside HTML tags where whitespaces are significant (ex. `<pre>`, * - text nodes are left as-is inside HTML tags where whitespaces are significant (ex. `<pre>`,
* `<textarea>`). * `<textarea>`).
* *
* Described transformations can (potentially) influence DOM nodes layout so the * Described transformations may (potentially) influence DOM nodes layout. However, the impact
* `preserveWhitespaces` option is `true` be default (no whitespace removal). * should so be minimal. That's why starting from Angular 6, the
* In Angular 5 you need to opt-in for whitespace removal (but we might revisit the default * `preserveWhitespaces` option is `false` by default (whitespace removal).
* setting in Angular 6 or later). If you want to change the default setting for all components * If you want to change the default setting for all components in your application you can use
* in your application you can use the `preserveWhitespaces` option of the AOT compiler. * 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 * in certain fragments of a template. You can either exclude entire DOM sub-tree by using the
* `ngPreserveWhitespaces` attribute, ex.: * `ngPreserveWhitespaces` attribute, ex.:
* *

View File

@ -191,9 +191,7 @@ class TestApp {
it('should list all child nodes', () => { it('should list all child nodes', () => {
fixture = TestBed.createComponent(ParentComp); fixture = TestBed.createComponent(ParentComp);
fixture.detectChanges(); fixture.detectChanges();
expect(fixture.debugElement.childNodes.length).toEqual(3);
// The root component has 3 elements and 2 text node children.
expect(fixture.debugElement.childNodes.length).toEqual(5);
}); });
it('should list all component child elements', () => { it('should list all component child elements', () => {

View File

@ -1280,7 +1280,7 @@ function declareTests({useJit}: {useJit: boolean}) {
fixture.detectChanges(); fixture.detectChanges();
expect(fixture.nativeElement) expect(fixture.nativeElement)
.toHaveText( .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); const f = TestBed.configureTestingModule({declarations: [MyCmp]}).createComponent(MyCmp);
f.detectChanges(); 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(() => { it('should not remove whitespaces when explicitly requested not to do so', async(() => {

View File

@ -123,7 +123,7 @@ class BadTemplateUrl {
TestBed.compileComponents().then(() => { TestBed.compileComponents().then(() => {
const componentFixture = TestBed.createComponent(ExternalTemplateComp); const componentFixture = TestBed.createComponent(ExternalTemplateComp);
componentFixture.detectChanges(); 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 10000); // Long timeout here because this test makes an actual ResourceLoader request, and

View File

@ -307,7 +307,7 @@ class CompWithUrlTemplate {
it('should allow to createSync components with templateUrl after explicit async compilation', it('should allow to createSync components with templateUrl after explicit async compilation',
() => { () => {
const fixture = TestBed.createComponent(CompWithUrlTemplate); const fixture = TestBed.createComponent(CompWithUrlTemplate);
expect(fixture.nativeElement).toHaveText('from external template\n'); expect(fixture.nativeElement).toHaveText('from external template');
}); });
}); });