fix(component): untrack subscription in ngrxPush pipe (#3918)

ngrx · May 31, 2023 · a1688e4 · a1688e4
1 parent 122b5f7
commit a1688e4
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 16 deletions.
diff --git a/modules/component/spec/push/push.pipe.spec.ts b/modules/component/spec/push/push.pipe.spec.ts
@@ -1,12 +1,26 @@
-import { ChangeDetectorRef, Component, ErrorHandler } from '@angular/core';
+import {
+  ChangeDetectorRef,
+  Component,
+  ErrorHandler,
+  computed,
+  signal,
+} from '@angular/core';
 import {
   ComponentFixture,
   fakeAsync,
   flushMicrotasks,
   TestBed,
   waitForAsync,
 } from '@angular/core/testing';
-import { BehaviorSubject, delay, EMPTY, NEVER, of, throwError } from 'rxjs';
+import {
+  BehaviorSubject,
+  delay,
+  EMPTY,
+  NEVER,
+  Observable,
+  of,
+  throwError,
+} from 'rxjs';
 import { PushPipe } from '../../src/push/push.pipe';
 import { MockChangeDetectorRef, MockErrorHandler } from '../fixtures/fixtures';
 import { stripSpaces, wrapWithSpace } from '../helpers';
@@ -157,6 +171,29 @@ describe('PushPipe', () => {
         expect(pushPipe.transform(100)).toBe(100);
         expect(pushPipe.transform('ngrx')).toBe('ngrx');
       });
+
+      it('should not track signal reads in subscriptions', () => {
+        const trigger = signal(false);
+
+        const obs = new Observable(() => {
+          // Whenever `obs` is subscribed, synchronously read `trigger`.
+          trigger();
+        });
+
+        let trackCount = 0;
+        const tracker = computed(() => {
+          // Subscribe to `obs` within this `computed`. If the subscription side effect runs
+          // within the computed, then changes to `trigger` will invalidate this computed.
+          pushPipe.transform(obs);
+
+          // The computed returns how many times it's run.
+          return ++trackCount;
+        });
+
+        expect(tracker()).toBe(1);
+        trigger.set(true);
+        expect(tracker()).toBe(1);
+      });
     });
   });
 

diff --git a/modules/component/src/core/render-event/manager.ts b/modules/component/src/core/render-event/manager.ts
@@ -6,6 +6,7 @@ import {
   fromPotentialObservable,
   PotentialObservableResult,
 } from '../potential-observable';
+import { untracked } from '@angular/core';
 
 export interface RenderEventManager<PO> {
   nextPotentialObservable(potentialObservable: PO): void;
@@ -46,20 +47,22 @@ function switchMapToRenderEvent<PO>(): (
 
       return new Observable<RenderEvent<PotentialObservableResult<PO>>>(
         (subscriber) => {
-          const subscription = observable$.subscribe({
-            next(value) {
-              subscriber.next({ type: 'next', value, reset, synchronous });
-              reset = false;
-            },
-            error(error) {
-              subscriber.next({ type: 'error', error, reset, synchronous });
-              reset = false;
-            },
-            complete() {
-              subscriber.next({ type: 'complete', reset, synchronous });
-              reset = false;
-            },
-          });
+          const subscription = untracked(() =>
+            observable$.subscribe({
+              next(value) {
+                subscriber.next({ type: 'next', value, reset, synchronous });
+                reset = false;
+              },
+              error(error) {
+                subscriber.next({ type: 'error', error, reset, synchronous });
+                reset = false;
+              },
+              complete() {
+                subscriber.next({ type: 'complete', reset, synchronous });
+                reset = false;
+              },
+            })
+          );
 
           if (reset) {
             subscriber.next({ type: 'suspense', reset, synchronous: true });