Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

perf(core): usengDevMode to tree-shakecheckNoChanges#39964

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
arturovt wants to merge8 commits intoangular:masterfromarturovt:guard-check-no-changes-with-ng-dev
Closed

perf(core): usengDevMode to tree-shakecheckNoChanges#39964

arturovt wants to merge8 commits intoangular:masterfromarturovt:guard-check-no-changes-with-ng-dev

Conversation

@arturovt
Copy link
Contributor

@arturovtarturovt commentedDec 3, 2020
edited
Loading

PR Checklist

PR Type

  • Refactoring (no functional changes, no api changes)

Does this PR introduce a breaking change?

  • Yes
  • No

@pullapprovepullapprovebot requested a review fromalxhubDecember 3, 2020 23:03
@AndrewKushnir
Copy link
Contributor

AndrewKushnir commentedDec 3, 2020
edited
Loading

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.

alan-agius4 reacted with thumbs up emoji

@AndrewKushnir
Copy link
Contributor

@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
Copy link
ContributorAuthor

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

AndrewKushnir reacted with thumbs up emoji

This commit adds `ngDevMode` guard to run `checkNoChanges` onlyin 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 onlyin 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 onlyin 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` onlyin 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.
Copy link
Contributor

@AndrewKushnirAndrewKushnir left a comment

Choose a reason for hiding this comment

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

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 useisDevMode() check. Could you please have a look when you get a chance?

@AndrewKushnirAndrewKushnir added action: reviewThe PR is still awaiting reviews from at least one requested reviewer area: commonIssues related to APIs in the @angular/common package area: coreIssues related to the framework runtime area: forms area: performanceIssues related to performance target: patchThis PR is targeted for the next patch release labelsDec 4, 2020
@ngbotngbotbot modified the milestone:BacklogDec 4, 2020
Copy link
Contributor

@AndrewKushnirAndrewKushnir left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the feedback@arturovt 👍

arturovt reacted with heart emoji
@mheverymhevery added the action: mergeThe PR is ready for merge by the caretaker labelDec 4, 2020
@AndrewKushnir
Copy link
Contributor

Presubmit.

@AndrewKushnirAndrewKushnir removed the action: reviewThe PR is still awaiting reviews from at least one requested reviewer labelDec 4, 2020
@AndrewKushnirAndrewKushnir added the action: presubmitThe PR is in need of a google3 presubmit labelDec 4, 2020
Copy link
Contributor

@mheverymhevery left a comment

Choose a reason for hiding this comment

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

reviewed-for: global-approvers

@mheverymhevery removed the action: presubmitThe PR is in need of a google3 presubmit labelDec 5, 2020
@mheverymhevery added target: minorThis PR is targeted for the next minor release and removed target: patchThis PR is targeted for the next patch release labelsDec 5, 2020
mhevery pushed a commit that referenced this pull requestDec 5, 2020
This commit adds ngDevMode guard to show warnings onlyin 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.PRClose#39964
mhevery pushed a commit that referenced this pull requestDec 5, 2020
This commit adds ngDevMode guard to show warning onlyin 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.PRClose#39964
mhevery pushed a commit that referenced this pull requestDec 5, 2020
This commit adds `ngDevMode` guard to call `_ngModelWarning` onlyin 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.PRClose#39964
@arturovtarturovt deleted the guard-check-no-changes-with-ng-dev branchDecember 5, 2020 00:08
@mheverymhevery added target: patchThis PR is targeted for the next patch release and removed target: minorThis PR is targeted for the next minor release labelsDec 5, 2020
mhevery pushed a commit that referenced this pull requestDec 5, 2020
This commit adds `ngDevMode` guard to run `checkNoChanges` onlyin 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.PRClose#39964
mhevery pushed a commit that referenced this pull requestDec 5, 2020
This commit adds ngDevMode guard to show warnings onlyin 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.PRClose#39964
mhevery pushed a commit that referenced this pull requestDec 5, 2020
This commit adds ngDevMode guard to show warning onlyin 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.PRClose#39964
mhevery pushed a commit that referenced this pull requestDec 5, 2020
This commit adds `ngDevMode` guard to call `_ngModelWarning` onlyin 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.PRClose#39964
@angular-automatic-lock-bot

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 ourautomatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-botangular-automatic-lock-botbot locked and limited conversation to collaboratorsJan 5, 2021
@pullapprovepullapprovebot removed area: commonIssues related to APIs in the @angular/common package area: coreIssues related to the framework runtime area: forms area: performanceIssues related to performance labelsJan 5, 2021
@ngbotngbotbot removed this from theBacklog milestoneJan 5, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@AndrewKushnirAndrewKushnirAndrewKushnir approved these changes

+1 more reviewer

@mheverymheverymhevery approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: mergeThe PR is ready for merge by the caretakercla: yestarget: patchThis PR is targeted for the next patch release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@arturovt@AndrewKushnir@mhevery

[8]ページ先頭

©2009-2025 Movatter.jp