Skip to content

build(zone.js): update zone.js to 0.10.2#31975

Closed
JiaLiPassion wants to merge 1 commit intoangular:masterfrom
JiaLiPassion:zone-update
Closed

build(zone.js): update zone.js to 0.10.2#31975
JiaLiPassion wants to merge 1 commit intoangular:masterfrom
JiaLiPassion:zone-update

Conversation

@JiaLiPassion
Copy link
Copy Markdown
Contributor

@JiaLiPassion JiaLiPassion commented Aug 3, 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: #31896

What is the new behavior?

update zone.js dependency to .0.10.1, will begin to work on remove zone.js from package.json, need to reference from source.

Does this PR introduce a breaking change?

  • Yes
  • No

@JiaLiPassion JiaLiPassion requested review from a team August 3, 2019 01:00
@JiaLiPassion JiaLiPassion force-pushed the zone-update branch 3 times, most recently from a70eacd to 2e45208 Compare August 3, 2019 03:27
@gkalpak gkalpak 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 Aug 3, 2019
gkalpak
gkalpak approved these changes Aug 3, 2019
View reviewed changes
Comment thread aio/scripts/_payload-limits.json Outdated
@JiaLiPassion JiaLiPassion removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Aug 3, 2019
sod
sod reviewed Aug 5, 2019
View reviewed changes
Comment thread aio/package.json Outdated
Copy link
Copy Markdown
Contributor

@sod sod Aug 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Have you tested this in a browser that requires the legacy polyfills? Right now the 0.10.1 zone-legacy.js file calls methods from zone.js itself:

image

But the angular-cli embeds them the other way around. First zone-legacy.js, then zone.js. So zone-legacy.js just fails because of missing Zone.

And zone-evergreen.js calls the patch:
image
It's not how it's documented in the zone.js readme, but I guess it's done this way so all patches and legacy things can be bundled up in a single es6-polyfills.js file.

Copy link
Copy Markdown
Contributor Author

@JiaLiPassion JiaLiPassion Aug 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@sod, thanks for posting the issue, I will fix this one.
And could you tell me the angular cli version you are using? Thanks!

@JiaLiPassion JiaLiPassion 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 Aug 6, 2019
@JiaLiPassion
Copy link
Copy Markdown
Contributor Author

JiaLiPassion commented Aug 6, 2019

need to fix the issue @sod found first.

@JiaLiPassion JiaLiPassion mentioned this pull request Aug 6, 2019
Closed
14 tasks
@alxhub alxhub added the area: zones Issues related to zone.js label Aug 8, 2019
@ngbot ngbot Bot added this to the needsTriage milestone Aug 8, 2019
@JiaLiPassion JiaLiPassion changed the title build(zone.js): update zone.js to 0.10.1 build(zone.js): update zone.js to 0.10.2 Aug 14, 2019
@JiaLiPassion JiaLiPassion force-pushed the zone-update branch 3 times, most recently from 7bd3107 to 56314cc Compare August 14, 2019 23:13
@JiaLiPassion JiaLiPassion removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Aug 14, 2019
gkalpak
gkalpak approved these changes Aug 15, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

One nit. LGTM otherwise 👍

Comment thread integration/bazel/package.json Outdated
Copy link
Copy Markdown
Member

@gkalpak gkalpak Aug 15, 2019

Choose a reason for hiding this comment

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

Why not 0.10.2?

Copy link
Copy Markdown
Contributor Author

@JiaLiPassion JiaLiPassion Aug 15, 2019

Choose a reason for hiding this comment

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

@gkalpak , thanks for the review, I have updated this one to 0.10.2 too.

mhevery
mhevery approved these changes Aug 15, 2019
View reviewed changes
Comment thread integration/bazel/yarn.lock Outdated
Copy link
Copy Markdown
Contributor

@mhevery mhevery Aug 15, 2019

Choose a reason for hiding this comment

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

Why are we going from 8.2 to 8.1?

Copy link
Copy Markdown
Contributor Author

@JiaLiPassion JiaLiPassion Aug 15, 2019

Choose a reason for hiding this comment

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

@mhevery , yes, you are right, and when I run a full rebuild, it seems the version will become the most latest version 9.0.0-xxx, I have reverted the changes here and only update zone.js version.

@JiaLiPassion JiaLiPassion force-pushed the zone-update branch 2 times, most recently from 2d72b6a to d007313 Compare August 15, 2019 23:00
@JiaLiPassion
build(zone.js): update zone.js to 0.10.2
4d95819 
Bundle size changed in both zone.js(legacy) and zone-evergreen.js

- zone.js(legacy) package increased a little because the following feature and fixes.
1. angular#31699, handle MSPointer events PR
2. angular/zone.js#1219 to add __zone_symbol__ customization support

- zone-evergreen.js package decreased because
1. the MSPointer PR only for legacy
2. the Object.defineProperty patch is moved to legacy angular#31660
@mhevery mhevery added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release labels Aug 16, 2019
sabeersulaiman pushed a commit to sabeersulaiman/angular that referenced this pull request Sep 6, 2019
@JiaLiPassion @sabeersulaiman
build(zone.js): update zone.js to 0.10.2 (angular#31975)
6b2fa49 
Bundle size changed in both zone.js(legacy) and zone-evergreen.js

- zone.js(legacy) package increased a little because the following feature and fixes.
1. angular#31699, handle MSPointer events PR
2. angular/zone.js#1219 to add __zone_symbol__ customization support

- zone-evergreen.js package decreased because
1. the MSPointer PR only for legacy
2. the Object.defineProperty patch is moved to legacy angular#31660

PR Close angular#31975
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot Bot commented Sep 16, 2019

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 Sep 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@sod sod sod left review comments

@mhevery mhevery mhevery approved these changes

@gkalpak gkalpak gkalpak approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: zones Issues related to zone.js cla: yes target: major This PR is targeted for the next major release

Projects

None yet

Milestone

needsTriage

Development

Successfully merging this pull request may close these issues.

6 participants

@JiaLiPassion @mhevery @sod @gkalpak @alxhub @googlebot