-
-
Notifications
You must be signed in to change notification settings - Fork 176
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
fix: 🐛 component re-render when updating fields in lifecycle hook #646
fix: 🐛 component re-render when updating fields in lifecycle hook #646
Conversation
|
Seeing 1 test fail which did succeed locally - will investigate when I have time after work. |
@NetanelBasal could you check this failing CI run? The failing test succeeds locally, I wonder if the flakiness is due to CI. One weird thing I'm seeing is that the CI job runs 290 tests, while locally I only have 280 tests 🤔 |
Maybe there is a skip some where? |
5cfe226
to
1820c3f
Compare
Got it, I simply didn't rebase on latest |
`, | ||
changeDetection: ChangeDetectionStrategy.OnPush, | ||
}) | ||
export class NgOnChangesInputComponent { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export class NgOnChangesInputComponent { | |
export class OnPushComponent { |
I think thats more to the point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, but let's not wait for this fix
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
See #643
When Spectator is used to set inputs on a component, the following will happen:
ChangeDetectorRef.detectChanges()
is calledfixture.detectChanges
is calledIssue Number: #643
What is the new behavior?
I simply swapped these 2 calls:
fixture.detectChanges
will run a change detection cycle on the test fixture. This is whenngOnChanges
will runChangeDetectorRef.detectChanges()
will check the component view. This is where the change detection check will detect the updated field fromngOnChanges
and re-render the component.I wish I could give a better explanation to why this happens, but this is my interpretation of the bug/fix
Does this PR introduce a breaking change?
Other information