Skip to content

Commit

Permalink
refactor(core): Do not update firstUpdatePass in checkNoChanges mode
Browse files Browse the repository at this point in the history
We don't want components to behave differently depending on whether check no changes.
This commit consolidates the remainder of the operations that should not
be done in checkNoChanges pass under a single `if` block where possible.
  • Loading branch information
atscott committed Oct 25, 2023
1 parent 5ebd17e commit 7e28fd0
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 17 deletions.
33 changes: 16 additions & 17 deletions packages/core/src/render3/instructions/change_detection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,24 +233,23 @@ export function refreshView<T>(
}
incrementInitPhaseFlags(lView, InitPhaseState.AfterViewInitHooksToBeRun);
}
}
if (tView.firstUpdatePass === true) {
// We need to make sure that we only flip the flag on successful `refreshView` only
// Don't do this in `finally` block.
// If we did this in `finally` block then an exception could block the execution of styling
// instructions which in turn would be unable to insert themselves into the styling linked
// list. The result of this would be that if the exception would not be throw on subsequent CD
// the styling would be unable to process it data and reflect to the DOM.
tView.firstUpdatePass = false;
}

// Do not reset the dirty state when running in check no changes mode. We don't want components
// to behave differently depending on whether check no changes is enabled or not. For example:
// Marking an OnPush component as dirty from within the `ngAfterViewInit` hook in order to
// refresh a `NgClass` binding should work. If we would reset the dirty state in the check
// no changes cycle, the component would be not be dirty for the next update pass. This would
// be different in production mode where the component dirty state is not reset.
if (!isInCheckNoChangesPass) {
if (tView.firstUpdatePass === true) {
// We need to make sure that we only flip the flag on successful `refreshView` only
// Don't do this in `finally` block.
// If we did this in `finally` block then an exception could block the execution of styling
// instructions which in turn would be unable to insert themselves into the styling linked
// list. The result of this would be that if the exception would not be throw on subsequent
// CD the styling would be unable to process it data and reflect to the DOM.
tView.firstUpdatePass = false;
}

// Do not reset the dirty state when running in check no changes mode. We don't want
// components to behave differently depending on whether check no changes is enabled or not.
// For example: Marking an OnPush component as dirty from within the `ngAfterViewInit` hook in
// order to refresh a `NgClass` binding should work. If we would reset the dirty state in the
// check no changes cycle, the component would be not be dirty for the next update pass. This
// would be different in production mode where the component dirty state is not reset.
lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass);
}
} catch (e) {
Expand Down
1 change: 1 addition & 0 deletions packages/core/test/linker/query_integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,7 @@ describe('Query API', () => {
const template =
'<auto-projecting #q><ng-template><div text="1"></div></ng-template></auto-projecting>';
const view = createTestCmpAndDetectChanges(MyComp0, template);
view.detectChanges();

const q = view.debugElement.children[0].references!['q'];
expect(q.query.length).toBe(1);
Expand Down

0 comments on commit 7e28fd0

Please sign in to comment.