refactor(zone.js): remove onProp eventName array to reduce bundle size #40962

JiaLiPassion · 2021-02-23T13:54:49Z

Zone.js supports google closure compiler in advance optimization mode,
to prevent closure compiler modify the onProperty name such as Element.prototype.onclick,
Zone.js implements the onProperty patch logic by declaring all the
event names in source code, this increases the bundle size and also require
updating the event names array to keep the information updated.

Now google closure compiler has the required event names defined in the built-in
externs files, so zone.js can use more simple implementation and decrease the bundle size
of zone.js. (about 3k)

josephperrott

LGTM

Looks like we just need the size tracking to be updated.

AndrewKushnir

Great refactoring and almost 4Kb savings! 👍

Reviewed-for: size-tracking

AndrewKushnir · 2021-02-24T00:42:02Z

@JiaLiPassion it looks like there are more places where payload size should be updated, see test_ivy_aot CI job output.

Could you please also set the "target" label (would it be safe to land in patch version or only in major)?

JiaLiPassion · 2021-02-24T01:16:26Z

@AndrewKushnir , @josephperrott , thank you for the review. I have updated the size and also added the target label.
@AndrewKushnir , could you also run a presubmit to check whether it breaks g3, I remembered there was an issue about using closure complier advanced optimization failed without these event names.

AndrewKushnir · 2021-02-24T01:33:23Z

@JiaLiPassion sure, I've started a presubmit, will let you know how it goes.

AndrewKushnir · 2021-02-24T06:29:38Z

@JiaLiPassion FYI presubmits went well, but I think we'd need to run a global one before merging this change. Thank you.

JiaLiPassion · 2021-02-24T06:43:28Z

@AndrewKushnir , got it, thank you!

petebacondarwin

Reviewed-for: size-tracking

packages/zone.js/lib/browser/api-util.ts

packages/zone.js/lib/browser/property-descriptor.ts

JiaLiPassion · 2022-02-10T06:45:19Z

@AndrewKushnir, got it, thank you very much!

packages/zone.js/lib/browser/property-descriptor-legacy.ts

alxhub · 2022-02-25T21:58:39Z

packages/zone.js/lib/browser/property-descriptor.ts

Can you add a doc comment to this function describing its intention (and specifically, that it only considers the immediate object properties and not any inherited properties).

JiaLiPassion · 2022-02-25T22:33:57Z

@alxhub , thank you for the review, I have added the comments and update the code style.

…e size Zone.js supports the google closure compiler in the advance optimization mode, to prevent closure compiler modify the `onProperty` name such as `Element.prototype.onclick`, Zone.js implements the `onProperty` patch logic by declaring all the event names in the source code, this increases the bundle size and also require updating the event names array to keep the information updated. Now google closure compiler has the required event names defined in the built-in externs files, so zone.js can use more simple implementation and decrease the bundle size of zone.js (about 4k).

For legacy browsers, we still use the eventNames array to patch event instead of using `Object.getOwnPropertyNames()` to keep backward compatibility.

AndrewKushnir · 2022-03-01T05:19:33Z

Global Presubmit #2.

AndrewKushnir · 2022-03-01T17:09:53Z

FYI, the presubmit is "green", so I'm adding this PR to the merge queue.

AndrewKushnir · 2022-03-01T17:11:21Z

Note to the Caretaker: please sync this PR into g3 as a separate change.

jessicajaniuk · 2022-03-01T18:39:58Z

This does not merge cleanly with 13.2.x. I'll update to target: minor and land it. Please create a patch PR targeting 13.2.x and we'll land that one, too.

jessicajaniuk · 2022-03-01T18:41:17Z

This PR was merged into the repository by commit 1b85811.

…es (#40962) For legacy browsers, we still use the eventNames array to patch event instead of using `Object.getOwnPropertyNames()` to keep backward compatibility. PR Close #40962

angular-automatic-lock-bot · 2022-04-01T16:11:36Z

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.}

…e size (angular#40962) Zone.js supports the google closure compiler in the advance optimization mode, to prevent closure compiler modify the `onProperty` name such as `Element.prototype.onclick`, Zone.js implements the `onProperty` patch logic by declaring all the event names in the source code, this increases the bundle size and also require updating the event names array to keep the information updated. Now google closure compiler has the required event names defined in the built-in externs files, so zone.js can use more simple implementation and decrease the bundle size of zone.js (about 4k). PR Close angular#40962

…es (angular#40962) For legacy browsers, we still use the eventNames array to patch event instead of using `Object.getOwnPropertyNames()` to keep backward compatibility. PR Close angular#40962

JiaLiPassion added area: build & ci Related the build and CI infrastructure of the project area: zones Issues related to zone.js labels Feb 23, 2021

JiaLiPassion requested review from josephperrott and mhevery February 23, 2021 13:54

ngbot bot modified the milestone: Backlog Feb 23, 2021

google-cla bot added the cla: yes label Feb 23, 2021

josephperrott approved these changes Feb 23, 2021

View reviewed changes

JiaLiPassion force-pushed the zone-externs-events branch from 76e8a7f to 41bfd50 Compare February 24, 2021 00:28

pullapprove bot requested review from AndrewKushnir, alxhub, atscott and petebacondarwin February 24, 2021 00:28

AndrewKushnir approved these changes Feb 24, 2021

View reviewed changes

pullapprove bot requested a review from jelbourn February 24, 2021 00:39

AndrewKushnir 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 24, 2021

JiaLiPassion force-pushed the zone-externs-events branch 2 times, most recently from a93240f to f88f609 Compare February 24, 2021 01:08

JiaLiPassion added the target: patch This PR is targeted for the next patch release label Feb 24, 2021

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 Feb 24, 2021

AndrewKushnir added the action: presubmit The PR is in need of a google3 presubmit label Feb 24, 2021

petebacondarwin approved these changes Feb 24, 2021

View reviewed changes

jelbourn reviewed Feb 24, 2021

View reviewed changes

JiaLiPassion force-pushed the zone-externs-events branch 2 times, most recently from f3eb998 to 939c525 Compare February 24, 2021 23:59

JiaLiPassion force-pushed the zone-externs-events branch 2 times, most recently from 581ed79 to 17d40b6 Compare February 25, 2022 21:46

alxhub approved these changes Feb 25, 2022

View reviewed changes

JiaLiPassion force-pushed the zone-externs-events branch from 17d40b6 to abbc31e Compare February 25, 2022 22:33

JiaLiPassion force-pushed the zone-externs-events branch 2 times, most recently from 98bd50a to 044b343 Compare February 25, 2022 23:18

JiaLiPassion force-pushed the zone-externs-events branch from 044b343 to cbea351 Compare March 1, 2022 03:58

refactor(zone.js): for legacy browser, still use hard coding eventNames

22d8b18

For legacy browsers, we still use the eventNames array to patch event instead of using `Object.getOwnPropertyNames()` to keep backward compatibility.

JiaLiPassion force-pushed the zone-externs-events branch from cbea351 to 22d8b18 Compare March 1, 2022 04:17

jessicajaniuk added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Mar 1, 2022

jessicajaniuk closed this in 0f298a1 Mar 1, 2022

JiaLiPassion mentioned this pull request Mar 2, 2022

refactor(zone.js): remove onProp eventNames array to reduce the bundle size #45230

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 1, 2022

refactor(zone.js): remove onProp eventName array to reduce bundle size #40962

refactor(zone.js): remove onProp eventName array to reduce bundle size #40962

Uh oh!

Conversation

JiaLiPassion commented Feb 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josephperrott left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir commented Feb 24, 2021

Uh oh!

JiaLiPassion commented Feb 24, 2021

Uh oh!

AndrewKushnir commented Feb 24, 2021

Uh oh!

AndrewKushnir commented Feb 24, 2021

Uh oh!

JiaLiPassion commented Feb 24, 2021

Uh oh!

petebacondarwin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JiaLiPassion commented Feb 10, 2022

Uh oh!

Uh oh!

alxhub Feb 25, 2022

Choose a reason for hiding this comment

Uh oh!

JiaLiPassion commented Feb 25, 2022

Uh oh!

AndrewKushnir commented Mar 1, 2022

Uh oh!

AndrewKushnir commented Mar 1, 2022

Uh oh!

AndrewKushnir commented Mar 1, 2022

Uh oh!

jessicajaniuk commented Mar 1, 2022

Uh oh!

jessicajaniuk commented Mar 1, 2022

Uh oh!

angular-automatic-lock-bot bot commented Apr 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

JiaLiPassion commented Feb 23, 2021 •

edited

Loading