perf(core): use `ngDevMode` to tree-shake `checkNoChanges` by arturovt · Pull Request #39964 · angular/angular

arturovt · 2020-12-03T23:03:01Z

PR Checklist

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

Refactoring (no functional changes, no api changes)

Does this PR introduce a breaking change?

Yes
No

AndrewKushnir · 2020-12-03T23:20:43Z

Hi @arturovt, thanks for creating PRs to improve tree-shaking 👍

I'd like to propose an idea to group all commits from other PRs (except the one that is already marked for merge) into a single PR (while keeping changes in separate commits). That should help review these changes as a whole and would simplify the process of landing these changes.

Thank you.

AndrewKushnir · 2020-12-03T23:25:00Z

@arturovt another benefit is that you may need to update payload sizes for test/demo apps (reducing the size), so there might be code conflicts that you would need to resolve multiple times...

arturovt · 2020-12-04T09:21:25Z

@arturovt another benefit is that you may need to update payload sizes for test/demo apps (reducing the size), so there might be code conflicts that you would need to resolve multiple times...

Yeah that makes sense. I was just thinking that it's a bad idea to have changes, that affect different packages, in 1 PR.

This commit adds `ngDevMode` guard to run `checkNoChanges` only in dev mode (similar to how things work in other parts of Ivy runtime code). The `ngDevMode` flag helps to tree-shake this code from production builds (in dev mode everything will work as it works right now) to decrease production bundle size.

This commit adds ngDevMode guard to show warnings only in dev mode (similar to how things work in other parts of Ivy runtime code). The ngDevMode flag helps to tree-shake these warnings from production builds (in dev mode everything will work as it works right now) to decrease production bundle size.

This commit adds ngDevMode guard to show warning only in dev mode (similar to how things work in other parts of Ivy runtime code). The ngDevMode flag helps to tree-shake this warning from production builds (in dev mode everything will work as it works right now) to decrease production bundle size.

This commit adds `ngDevMode` guard to call `_ngModelWarning` only in dev mode (similar to how things work in other parts of Ivy runtime code). The `ngDevMode` flag helps to tree-shake this function from production builds (since it will act as no-op, in dev mode everything will work as it works right now) to decrease production bundle size.

AndrewKushnir

Thanks for grouping commits into a single PR @arturovt 👍

The changes look good, just left a comment were I believe we'd still need to use isDevMode() check. Could you please have a look when you get a chance?

packages/core/src/application_ref.ts

AndrewKushnir

Thanks for addressing the feedback @arturovt 👍

AndrewKushnir · 2020-12-04T23:11:36Z

Presubmit.

mhevery

reviewed-for: global-approvers

This commit adds ngDevMode guard to show warnings only in dev mode (similar to how things work in other parts of Ivy runtime code). The ngDevMode flag helps to tree-shake these warnings from production builds (in dev mode everything will work as it works right now) to decrease production bundle size. PR Close #39964

This commit adds ngDevMode guard to show warning only in dev mode (similar to how things work in other parts of Ivy runtime code). The ngDevMode flag helps to tree-shake this warning from production builds (in dev mode everything will work as it works right now) to decrease production bundle size. PR Close #39964

This commit adds `ngDevMode` guard to call `_ngModelWarning` only in dev mode (similar to how things work in other parts of Ivy runtime code). The `ngDevMode` flag helps to tree-shake this function from production builds (since it will act as no-op, in dev mode everything will work as it works right now) to decrease production bundle size. PR Close #39964

This commit adds `ngDevMode` guard to run `checkNoChanges` only in dev mode (similar to how things work in other parts of Ivy runtime code). The `ngDevMode` flag helps to tree-shake this code from production builds (in dev mode everything will work as it works right now) to decrease production bundle size. PR Close #39964

This commit adds ngDevMode guard to show warnings only in dev mode (similar to how things work in other parts of Ivy runtime code). The ngDevMode flag helps to tree-shake these warnings from production builds (in dev mode everything will work as it works right now) to decrease production bundle size. PR Close #39964

This commit adds ngDevMode guard to show warning only in dev mode (similar to how things work in other parts of Ivy runtime code). The ngDevMode flag helps to tree-shake this warning from production builds (in dev mode everything will work as it works right now) to decrease production bundle size. PR Close #39964

This commit adds `ngDevMode` guard to call `_ngModelWarning` only in dev mode (similar to how things work in other parts of Ivy runtime code). The `ngDevMode` flag helps to tree-shake this function from production builds (since it will act as no-op, in dev mode everything will work as it works right now) to decrease production bundle size. PR Close #39964

angular-automatic-lock-bot · 2021-01-05T16:57:10Z

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

google-cla bot added the cla: yes label Dec 3, 2020

pullapprove bot requested a review from alxhub December 3, 2020 23:03

arturovt added 2 commits December 4, 2020 11:21

pullapprove bot requested a review from AndrewKushnir December 4, 2020 09:24

pullapprove bot requested a review from IgorMinar December 4, 2020 11:00

AndrewKushnir requested changes Dec 4, 2020

View reviewed changes

packages/core/src/application_ref.ts Outdated Show resolved Hide resolved

AndrewKushnir requested a review from mhevery December 4, 2020 20:29

ngbot bot modified the milestone: Backlog Dec 4, 2020

arturovt added 4 commits December 4, 2020 23:10

fixup! perf(core): use ngDevMode to tree-shake checkNoChanges

264f1a4

fixup! perf(core): use ngDevMode to tree-shake checkNoChanges

2d56c2b

fixup! perf(core): use ngDevMode to tree-shake checkNoChanges

2c6a6f1

fixup! perf(core): use ngDevMode to tree-shake checkNoChanges

fd71829

AndrewKushnir approved these changes Dec 4, 2020

View reviewed changes

mhevery added the action: merge The PR is ready for merge by the caretaker label Dec 4, 2020

AndrewKushnir removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Dec 4, 2020

AndrewKushnir added the action: presubmit The PR is in need of a google3 presubmit label Dec 4, 2020

mhevery approved these changes Dec 5, 2020

View reviewed changes

mhevery removed the action: presubmit The PR is in need of a google3 presubmit label Dec 5, 2020

mhevery removed request for IgorMinar and alxhub December 5, 2020 00:04

mhevery 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 Dec 5, 2020

mhevery closed this in e1fe9ec Dec 5, 2020

arturovt deleted the guard-check-no-changes-with-ng-dev branch December 5, 2020 00:08

mhevery added target: patch This PR is targeted for the next patch release and removed target: minor This PR is targeted for the next minor release labels Dec 5, 2020

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 5, 2021

pullapprove bot removed area: common Issues related to APIs in the @angular/common package area: core Issues related to the framework runtime area: forms area: performance Issues related to performance labels Jan 5, 2021

ngbot bot removed this from the Backlog milestone Jan 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf(core): use `ngDevMode` to tree-shake `checkNoChanges`#39964

perf(core): use `ngDevMode` to tree-shake `checkNoChanges`#39964
arturovt wants to merge 8 commits intoangular:masterfrom
arturovt:guard-check-no-changes-with-ng-dev

arturovt commented Dec 3, 2020 •

edited

Loading

Uh oh!

AndrewKushnir commented Dec 3, 2020 •

edited

Loading

Uh oh!

AndrewKushnir commented Dec 3, 2020

Uh oh!

arturovt commented Dec 4, 2020

Uh oh!

AndrewKushnir left a comment

Uh oh!

Uh oh!

AndrewKushnir left a comment

Uh oh!

AndrewKushnir commented Dec 4, 2020

Uh oh!

mhevery left a comment

Uh oh!

angular-automatic-lock-bot bot commented Jan 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

arturovt commented Dec 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

PR Type

Does this PR introduce a breaking change?

Uh oh!

AndrewKushnir commented Dec 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndrewKushnir commented Dec 3, 2020

Uh oh!

arturovt commented Dec 4, 2020

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir commented Dec 4, 2020

Uh oh!

mhevery left a comment

Choose a reason for hiding this comment

Uh oh!

angular-automatic-lock-bot bot commented Jan 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arturovt commented Dec 3, 2020 •

edited

Loading

AndrewKushnir commented Dec 3, 2020 •

edited

Loading