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

benbraou · 2018-02-06T19:36:16Z

Fixes #22027

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

preserveWhitespaces is true by default

What is the new behavior?

preserveWhitespaces is false by default

Does this PR introduce a breaking change?

[x] Yes
[ ] No

The impact of this breaking change seems to be already assessed as minimal as described in #22027

Other information

benbraou · 2018-02-06T19:41:25Z

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


Here (as well as in other tests) I have just updated the expected value rather then having two tests: one with preserveWhiteSpaces=true and preserveWhiteSpaces=false. I did not add the extra test because this does not seem to be the purpose of the existing test (createSync components).

benbraou · 2018-02-06T21:22:40Z

I get the error Commit 437aced2ab3cffa63ffa0707214d9b870b08462a uncompressed main exceeded expected size by >1% (expected: 459119, actual: 464506).
This is because I have removed from aio/src/tsconfig.app.json preserveWhitespaces setting. The aio project depends on angular 5.2.0 and not a the built version(where preserveWhitespaces is false by default). That's why, I will re-set aio/src/tsconfig.app.json. We may want to remove the option when aio migrates to Angular 6

mhevery · 2018-02-06T22:26:52Z

http://test/OCL:184739848:BASE:184740273:1517956003963:61235fa8

IgorMinar

can you just change that one doc please? the rest looks great. thanks!!

IgorMinar · 2018-02-07T00:17:38Z

aio/content/guide/aot-compiler.md

-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.
+This option is `false` by default, which results in smaller emitted template factory modules.


Maybe say, "As of v6, this option is false by default ..."

@IgorMinar done.

benbraou · 2018-02-07T01:03:00Z

aot-compiler.md updated as requested and commit amended

mhevery · 2018-02-09T21:18:06Z

http://test/OCL:184739848:BASE:185183195:1518211077966:cc147f29
global: https://test.corp.google.com/OCL:184739848:BASE:185183569:1518211250577:292458cc

ngbot · 2018-02-13T00:17:22Z

Hi @benbraou! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

benbraou · 2018-02-13T02:00:37Z

Merge conflict in aot-compiler.md fixed

Fixes angular#22027

benbraou · 2018-02-13T06:23:44Z

Weird... After rebase on top of master, the build job CI=ai_tools_test is failing (locally, aio tests are passing)

I see the error

Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL

I am not sure if just a rebuild is needed or a more serious problem is causing this.
The change in aot-compiler.md is pretty simple.
I will look at this tomorrow. Meanwhile, any hint would be great 😃

IgorMinar · 2018-02-13T06:59:53Z

I've restarted the ci job. let's see if we can repro the issue again.

benbraou · 2018-02-13T17:51:16Z

The CI build is now passing. Thank you!

IgorMinar · 2018-02-15T08:33:36Z

@mhevery I thought you tested this in g3 already? can we land it?

Fixes angular#22027 PR Close angular#22046

ahnpnl · 2018-03-01T07:44:09Z

hi @mhevery, I have a question regarding to this. Does TestBed also automatically get this option false by default ?

IgorMinar · 2018-03-02T23:10:01Z

@AhnpGit, it should. do you see something else? if so, please file an issue and provide a reproduction.

ahnpnl · 2018-03-02T23:16:59Z

hi @IgorMinar, it worked :) Nothing strange happens

IgorMinar · 2018-03-02T23:25:02Z

cool

Fixes angular#22027 PR Close angular#22046

angular-automatic-lock-bot · 2019-09-13T09:22:44Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Feb 6, 2018

benbraou commented Feb 6, 2018

View reviewed changes

benbraou force-pushed the issue22027 branch 2 times, most recently from 889d64b to 65ef710 Compare February 6, 2018 20:37

benbraou force-pushed the issue22027 branch from 65ef710 to 5ee92be Compare February 6, 2018 21:31

benbraou mentioned this pull request Feb 6, 2018

feat(core): turn whitespace removal on by default #22050

Closed

3 tasks

mhevery approved these changes Feb 6, 2018

View reviewed changes

mhevery self-assigned this Feb 6, 2018

mhevery added the target: major This PR is targeted for the next major release label Feb 6, 2018

mhevery requested a review from IgorMinar February 6, 2018 22:27

IgorMinar approved these changes Feb 7, 2018

View reviewed changes

IgorMinar added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Feb 7, 2018

benbraou force-pushed the issue22027 branch from 5ee92be to 0eb0639 Compare February 7, 2018 01:01

mhevery added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Feb 7, 2018

mhevery added the area: core Issues related to the framework runtime label Feb 13, 2018

feat(core): set preserveWhitespaces to false by default

6135515

Fixes angular#22027

benbraou force-pushed the issue22027 branch from e2023d3 to 6135515 Compare February 13, 2018 03:03

vicb added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Feb 15, 2018

vicb removed the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Feb 16, 2018

vicb closed this in f1a0632 Feb 16, 2018

benbraou deleted the issue22027 branch February 17, 2018 18:53

jbogarthyde pushed a commit to jbogarthyde/angular that referenced this pull request Feb 23, 2018

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

73850d4

Fixes angular#22027 PR Close angular#22046

smdunn pushed a commit to smdunn/angular that referenced this pull request Feb 28, 2018

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

f58a08f

Fixes angular#22027 PR Close angular#22046

leo6104 pushed a commit to leo6104/angular that referenced this pull request Mar 25, 2018

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

07bb45a

Fixes angular#22027 PR Close angular#22046

pfeigl mentioned this pull request May 19, 2018

material-buttons does not have default space between them in v6.0 as they have in v5.0 angular/components#11397

Closed

biiiipy mentioned this pull request May 23, 2018

V6 button boxes has no nature margin #24081

Closed

jenniferfell mentioned this pull request Jun 6, 2018

Missmatch v6 aot documentation and update.angular.io 5.2 to 6.0 #23854

Closed

jmleoni pushed a commit to jmleoni/angular that referenced this pull request Oct 6, 2018

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

3142add

Fixes angular#22027 PR Close angular#22046

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

benbraou commented Feb 6, 2018

benbraou Feb 6, 2018

benbraou commented Feb 6, 2018

mhevery commented Feb 6, 2018

IgorMinar left a comment

IgorMinar Feb 7, 2018

benbraou Feb 7, 2018

benbraou commented Feb 7, 2018 •

edited

Loading

mhevery commented Feb 9, 2018 •

edited

Loading

ngbot bot commented Feb 13, 2018

benbraou commented Feb 13, 2018

benbraou commented Feb 13, 2018

IgorMinar commented Feb 13, 2018

benbraou commented Feb 13, 2018

IgorMinar commented Feb 15, 2018

ahnpnl commented Mar 1, 2018

IgorMinar commented Mar 2, 2018 •

edited

Loading

ahnpnl commented Mar 2, 2018

IgorMinar commented Mar 2, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019

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

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

Conversation

benbraou commented Feb 6, 2018

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

benbraou Feb 6, 2018

Choose a reason for hiding this comment

benbraou commented Feb 6, 2018

mhevery commented Feb 6, 2018

IgorMinar left a comment

Choose a reason for hiding this comment

IgorMinar Feb 7, 2018

Choose a reason for hiding this comment

benbraou Feb 7, 2018

Choose a reason for hiding this comment

benbraou commented Feb 7, 2018 • edited Loading

mhevery commented Feb 9, 2018 • edited Loading

ngbot bot commented Feb 13, 2018

benbraou commented Feb 13, 2018

benbraou commented Feb 13, 2018

IgorMinar commented Feb 13, 2018

benbraou commented Feb 13, 2018

IgorMinar commented Feb 15, 2018

ahnpnl commented Mar 1, 2018

IgorMinar commented Mar 2, 2018 • edited Loading

ahnpnl commented Mar 2, 2018

IgorMinar commented Mar 2, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019

benbraou commented Feb 7, 2018 •

edited

Loading

mhevery commented Feb 9, 2018 •

edited

Loading

IgorMinar commented Mar 2, 2018 •

edited

Loading