refactor(core): Do no run queries in checkNoChanges

Quieries should not run in checkNoChanges mode. We don't want components to behave differently depending on whether check no changes is enabled or not.
atscott · Oct 25, 2023 · 5ebd17e · 5ebd17e
1 parent 76152a5
commit 5ebd17e
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 19 deletions.
diff --git a/packages/core/src/render3/instructions/change_detection.ts b/packages/core/src/render3/instructions/change_detection.ts
@@ -179,14 +179,14 @@ export function refreshView<T>(
     markTransplantedViewsForRefresh(lView);
     detectChangesInEmbeddedViews(lView, ChangeDetectionMode.Global);
 
-    // Content query results must be refreshed before content hooks are called.
-    if (tView.contentQueries !== null) {
-      refreshContentQueries(tView, lView);
-    }
-
-    // execute content hooks (AfterContentInit, AfterContentChecked)
-    // PERF WARNING: do NOT extract this to a separate function without running benchmarks
     if (!isInCheckNoChangesPass) {
+      // Content query results must be refreshed before content hooks are called.
+      if (tView.contentQueries !== null) {
+        refreshContentQueries(tView, lView);
+      }
+
+      // execute content hooks (AfterContentInit, AfterContentChecked)
+      // PERF WARNING: do NOT extract this to a separate function without running benchmarks
       if (hooksInitPhaseCompleted) {
         const contentCheckHooks = tView.contentCheckHooks;
         if (contentCheckHooks !== null) {
@@ -210,17 +210,17 @@ export function refreshView<T>(
       detectChangesInChildComponents(lView, components, ChangeDetectionMode.Global);
     }
 
-    // View queries must execute after refreshing child components because a template in this view
-    // could be inserted in a child component. If the view query executes before child component
-    // refresh, the template might not yet be inserted.
-    const viewQuery = tView.viewQuery;
-    if (viewQuery !== null) {
-      executeViewQueryFn<T>(RenderFlags.Update, viewQuery, context);
-    }
-
-    // execute view hooks (AfterViewInit, AfterViewChecked)
-    // PERF WARNING: do NOT extract this to a separate function without running benchmarks
     if (!isInCheckNoChangesPass) {
+      // View queries must execute after refreshing child components because a template in this view
+      // could be inserted in a child component. If the view query executes before child component
+      // refresh, the template might not yet be inserted.
+      const viewQuery = tView.viewQuery;
+      if (viewQuery !== null) {
+        executeViewQueryFn<T>(RenderFlags.Update, viewQuery, context);
+      }
+
+      // execute view hooks (AfterViewInit, AfterViewChecked)
+      // PERF WARNING: do NOT extract this to a separate function without running benchmarks
       if (hooksInitPhaseCompleted) {
         const viewCheckHooks = tView.viewCheckHooks;
         if (viewCheckHooks !== null) {

diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts
@@ -2488,14 +2488,16 @@ describe('runtime i18n', () => {
 
       // Create embedded view
       q.create();
-      fixture.detectChanges();
+      fixture.detectChanges(false);
+      fixture.detectChanges(false);
       expect(q.query.length).toEqual(1);
       expect(toHtml(fixture.nativeElement))
           .toEqual(`<div-query>Contenu<!--ng-container--></div-query>`);
 
       // Disable ng-if
       fixture.componentInstance.visible = false;
-      fixture.detectChanges();
+      fixture.detectChanges(false);
+      fixture.detectChanges(false);
       expect(q.query.length).toEqual(0);
       expect(toHtml(fixture.nativeElement))
           .toEqual(`<div-query>Contenu<!--ng-container--></div-query>`);