Skip to content

perf(core): allow checkNoChanges mode to be tree-shaken in production#45913

Closed
JoostK wants to merge 2 commits intoangular:mainfrom
JoostK:devmode-checknochanges
Closed

perf(core): allow checkNoChanges mode to be tree-shaken in production#45913
JoostK wants to merge 2 commits intoangular:mainfrom
JoostK:devmode-checknochanges

Conversation

@JoostK
Copy link
Member

@JoostK JoostK commented May 6, 2022

This commit guards all logic that exists for the checkNoChanges mode
with ngDevMode checks such that the logic can be tree-shaken.

This commit guards all logic that exists for the `checkNoChanges` mode
with `ngDevMode` checks such that the logic can be tree-shaken.
@JoostK JoostK force-pushed the devmode-checknochanges branch from 5f282b9 to 83f75f3 Compare May 6, 2022 17:40
@JoostK JoostK marked this pull request as ready for review May 6, 2022 18:03
@JoostK JoostK requested a review from AndrewKushnir May 6, 2022 18:03
@JoostK JoostK added action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release labels May 6, 2022
@ngbot ngbot bot added this to the Backlog milestone May 6, 2022
@JoostK
Copy link
Member Author

JoostK commented May 6, 2022

This saves 65514-65165=349 bytes in the todo app (see core/todo/bundle artifact of test job on CircleCI)

Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

@JoostK thanks for the improvement! 👍 I've added a few minor comments.

@JoostK JoostK force-pushed the devmode-checknochanges branch from bf4232a to ad3103a Compare May 7, 2022 20:01
@AndrewKushnir AndrewKushnir added action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels May 8, 2022
@AndrewKushnir
Copy link
Contributor

Presubmit.

@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker and removed action: presubmit The PR is in need of a google3 presubmit labels May 9, 2022
@ngbot
Copy link

ngbot bot commented May 9, 2022

I see that you just added the action: merge label, but the following checks are still failing:
    failure status "ci/circleci: aio_preview" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@AndrewKushnir AndrewKushnir added target: rc This PR is targeted for the next release-candidate and removed target: patch This PR is targeted for the next patch release labels May 9, 2022
@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit 75b3d0f.

AndrewKushnir pushed a commit that referenced this pull request May 9, 2022
…on (#45913)

This commit guards all logic that exists for the `checkNoChanges` mode
with `ngDevMode` checks such that the logic can be tree-shaken.

PR Close #45913
@AndrewKushnir
Copy link
Contributor

@JoostK there was a merge conflict with the 13.3.x branch, so this PR was merged into the 14.0.x and main branches only. Could you please create a new PR and target the 13.3.x branch if the change should be included there as well? Thank you.

@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 Jun 9, 2022
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: core Issues related to the framework runtime target: rc This PR is targeted for the next release-candidate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants