Permalink
Browse files

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

Fixes #22027

PR Close #22046
  • Loading branch information...
benbraou authored and vicb committed Feb 5, 2018
1 parent d241532 commit f1a063298e9ab4b30536e877e2d6ecc34873846f
@@ -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*
@@ -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()
}
})

@@ -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 */
@@ -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;
}
@@ -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);
});
});
}
@@ -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.:
*
@@ -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', () => {
@@ -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(() => {
@@ -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', () => {
@@ -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');
});
});

@@ -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
@@ -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');
});
});

@@ -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',

0 comments on commit f1a0632

Please sign in to comment.