Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(docs-infra): update @angular/cli 9.0.0-rc.0 and @angular/* to 9.0.0-rc.1 #33547

Closed
wants to merge 4 commits into from

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Nov 2, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • 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?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@mary-poppins
Copy link

You can preview 13acb2e at https://pr33547-13acb2e.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 406fbf7 at https://pr33547-406fbf7.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 85a8aa5 at https://pr33547-85a8aa5.ngbuilds.io/.

@gkalpak gkalpak changed the title build(docs-infra): update @angular/* and @angular/cli to 9.0.0-rc.0 build(docs-infra): update @angular/cli and @angular/* to 9.0.0-rc.0 Nov 2, 2019
@gkalpak gkalpak added area: build & ci Related the build and CI infrastructure of the project comp: docs-infra target: patch This PR is targeted for the next patch release labels Nov 2, 2019
@ngbot ngbot bot added this to the needsTriage milestone Nov 2, 2019
@gkalpak gkalpak marked this pull request as ready for review November 2, 2019 13:54
@gkalpak gkalpak requested review from a team as code owners November 2, 2019 13:54
Copy link
Contributor

@filipesilva filipesilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wanna try using --experimentalRollupPass? 😉

@@ -3,7 +3,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 2987,
"main-es2015": 460590,
"main-es2015": 463485,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3k greater :-/

@gkalpak
Copy link
Member Author

gkalpak commented Nov 7, 2019

Added another commit to update @angular/* to 9.0.0-rc.1. PTAL.

@mary-poppins
Copy link

You can preview e12873f at https://pr33547-e12873f.ngbuilds.io/.

@gkalpak
Copy link
Member Author

gkalpak commented Nov 7, 2019

@filipesilva, I tried --experimentalRollupPass, but I am getting the following error (and don't have time to look into it right now):

ERROR in ./src/search.service-f862c759.js
Module not found: Error: Can't resolve './search.worker' in '...\angular\aio\src'
resolve './search.worker' in '...\angular\aio\src'
  using description file: ...\angular\aio\package.json (relative path: ./src)
    Field 'browser' doesn't contain a valid alias configuration
    using description file: ...\angular\aio\package.json (relative path: ./src/search.worker)
      no extension
        Field 'browser' doesn't contain a valid alias configuration
        ...\angular\aio\src\search.worker doesn't exist
      .ts
        Field 'browser' doesn't contain a valid alias configuration
        ...\angular\aio\src\search.worker.ts doesn't exist
      .tsx
        Field 'browser' doesn't contain a valid alias configuration
        ...\angular\aio\src\search.worker.tsx doesn't exist
      .mjs
        Field 'browser' doesn't contain a valid alias configuration
        ...\angular\aio\src\search.worker.mjs doesn't exist
      .js
        Field 'browser' doesn't contain a valid alias configuration
        ...\angular\aio\src\search.worker.js doesn't exist
      as directory
        ...\angular\aio\src\search.worker doesn't exist
[...\angular\aio\src\search.worker]
[...\angular\aio\src\search.worker.ts]
[...\angular\aio\src\search.worker.tsx]
[...\angular\aio\src\search.worker.mjs]
[...\angular\aio\src\search.worker.js]
 @ ./src/search.service-f862c759.js 1:0-143
 @ ./src/main.ts
 @ multi ./src/main.ts

@filipesilva
Copy link
Contributor

@gkalpak these are the needed changes to make the option work: #33096

@gkalpak
Copy link
Member Author

gkalpak commented Nov 7, 2019

FYI:
With --experimentalRollupPass, the main bundle drops from 464973B to 449612B.
The total size of the lazy modules seems to be going up significantly (120086B -> 192837B ?), although it is hard to tell with the unnamed chunks.

@filipesilva
Copy link
Contributor

I expected a better improvement, will have to look into it,.

@gkalpak gkalpak changed the title build(docs-infra): update @angular/cli and @angular/* to 9.0.0-rc.0 build(docs-infra): update @angular/cli 9.0.0-rc.0 and @angular/* to 9.0.0-rc.1 Nov 11, 2019
@gkalpak gkalpak added the action: merge The PR is ready for merge by the caretaker label Nov 11, 2019
@@ -3,7 +3,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 2987,
"main-es2015": 460590,
"main-es2015": 464973,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gkalpak could you please test if this regression is coming from CLI changes or fw changes by reverting the cli update?

we should let this PR merge as is, but it would be good to know where the size regression came from.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed the size increase is caused by framework. (I got the exact same number after reverting the CLI update.)

Note that similar size regressions have been seen in integration projects recently (tracked in FW-1665). Not sure they are related, but I wouldn't be surprised 😃

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the CLI we also saw it in angular/angular-cli#16099, added potential causes there.

@kara kara closed this in 0260b8c Nov 11, 2019
kara pushed a commit that referenced this pull request Nov 11, 2019
kara pushed a commit that referenced this pull request Nov 11, 2019
kara pushed a commit that referenced this pull request Nov 11, 2019
kara pushed a commit that referenced this pull request Nov 11, 2019
kara pushed a commit that referenced this pull request Nov 11, 2019
kara pushed a commit that referenced this pull request Nov 11, 2019
kara pushed a commit that referenced this pull request Nov 11, 2019
@gkalpak gkalpak deleted the build-aio-update-angular branch November 11, 2019 19:31
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: build & ci Related the build and CI infrastructure of the project cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants