Skip to content

Use exhaustive checkNoChanges by default #31069

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented May 9, 2025

When components use OnPush, they prevent ExpressionHasChanged...
from being thrown when it should (angular/angular#45612).
The exhaustive option ignores dirty
flags and traverses the entire view tree checking for changes. This
commit configures component tests to do this by default and opts out any
tests with existing issues.

This also includes a commit to show one of the issues that surfaced along with the fix

When components use `OnPush`, they prevent `ExpressionHasChanged...`
from being thrown when it should (angular/angular#45612).
The exhaustive option ignores dirty
flags and traverses the entire view tree checking for changes. This
commit configures component tests to do this by default and opts out any
tests with existing issues.
@atscott atscott requested review from a team as code owners May 9, 2025 15:39
@atscott atscott requested review from adolgachev and mmalerba and removed request for a team May 9, 2025 15:39
@mmalerba mmalerba added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels May 9, 2025
@mmalerba
Copy link
Contributor

mmalerba commented May 9, 2025

@atscott can you run pnpm bazel run //goldens:cdk_api.accept to fix the api golden?

@atscott atscott 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, 2025
@atscott
Copy link
Contributor Author

atscott commented May 9, 2025

@mmalerba I've dropped the second commit now so it's a test-only change that won't need a presubmit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker 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