revert: fix(core): allow toSignal in reactive contexts

goldens/public-api/core/errors.md

@@ -19,6 +19,8 @@ export const enum RuntimeErrorCode {
     // (undocumented)
     APPLICATION_REF_ALREADY_DESTROYED = 406,
     // (undocumented)
+    ASSERTION_NOT_INSIDE_REACTIVE_CONTEXT = 602,
+    // (undocumented)
     ASYNC_INITIALIZERS_STILL_RUNNING = 405,
     // (undocumented)
     BOOTSTRAP_COMPONENTS_NOT_FOUND = -403,

goldens/public-api/core/index.md

@@ -128,6 +128,9 @@ export function asNativeElements(debugEls: DebugElement[]): any;
 // @public
 export function assertInInjectionContext(debugFn: Function): void;
 
+// @public
+export function assertNotInReactiveContext(debugFn: Function, extraContext?: string): void;
+
 // @public
 export function assertPlatform(requiredToken: any): PlatformRef;
 

packages/core/rxjs-interop/src/to_signal.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {assertInInjectionContext, computed, DestroyRef, inject, Injector, signal, Signal, untracked, WritableSignal, ɵRuntimeError, ɵRuntimeErrorCode} from '@angular/core';
+import {assertInInjectionContext, assertNotInReactiveContext, computed, DestroyRef, inject, Injector, signal, Signal, WritableSignal, ɵRuntimeError, ɵRuntimeErrorCode} from '@angular/core';
 import {Observable, Subscribable} from 'rxjs';
 
 /**
@@ -159,6 +159,12 @@ export function toSignal<T>(
     options: ToSignalOptions<undefined>&{requireSync: true}): Signal<T>;
 export function toSignal<T, U = undefined>(
     source: Observable<T>|Subscribable<T>, options?: ToSignalOptions<U>): Signal<T|U> {
+  ngDevMode &&
+      assertNotInReactiveContext(
+          toSignal,
+          'Invoking `toSignal` causes new subscriptions every time. ' +
+              'Consider moving `toSignal` outside of the reactive context and read the signal value where needed.');
+
   const requiresCleanup = !options?.manualCleanup;
   requiresCleanup && !options?.injector && assertInInjectionContext(toSignal);
   const cleanupRef =
@@ -175,24 +181,28 @@ export function toSignal<T, U = undefined>(
     state = signal<State<T|U>>({kind: StateKind.Value, value: options?.initialValue as U});
   }
 
-  untracked(() => {
-    const sub = source.subscribe({
-      next: value => state.set({kind: StateKind.Value, value}),
-      error: error => state.set({kind: StateKind.Error, error}),
-      // Completion of the Observable is meaningless to the signal. Signals don't have a concept of
-      // "complete".
-    });
-
-    if (ngDevMode && options?.requireSync && state().kind === StateKind.NoValue) {
-      throw new ɵRuntimeError(
-          ɵRuntimeErrorCode.REQUIRE_SYNC_WITHOUT_SYNC_EMIT,
-          '`toSignal()` called with `requireSync` but `Observable` did not emit synchronously.');
-    }
-
-    // Unsubscribe when the current context is destroyed, if requested.
-    cleanupRef?.onDestroy(sub.unsubscribe.bind(sub));
+  // Note: This code cannot run inside a reactive context (see assertion above). If we'd support
+  // this, we would subscribe to the observable outside of the current reactive context, avoiding
+  // that side-effect signal reads/writes are attribute to the current consumer. The current
+  // consumer only needs to be notified when the `state` signal changes through the observable
+  // subscription. Additional context (related to async pipe):
+  // https://github.com/angular/angular/pull/50522.
+  const sub = source.subscribe({
+    next: value => state.set({kind: StateKind.Value, value}),
+    error: error => state.set({kind: StateKind.Error, error}),
+    // Completion of the Observable is meaningless to the signal. Signals don't have a concept of
+    // "complete".
   });
 
+  if (ngDevMode && options?.requireSync && state().kind === StateKind.NoValue) {
+    throw new ɵRuntimeError(
+        ɵRuntimeErrorCode.REQUIRE_SYNC_WITHOUT_SYNC_EMIT,
+        '`toSignal()` called with `requireSync` but `Observable` did not emit synchronously.');
+  }
+
+  // Unsubscribe when the current context is destroyed, if requested.
+  cleanupRef?.onDestroy(sub.unsubscribe.bind(sub));
+
   // The actual returned signal is a `computed` of the `State` signal, which maps the various states
   // to either values or errors.
   return computed(() => {

packages/core/rxjs-interop/test/to_signal_spec.ts

@@ -6,8 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {computed, EnvironmentInjector, Injector, runInInjectionContext} from '@angular/core';
+import {ChangeDetectionStrategy, Component, computed, EnvironmentInjector, Injector, runInInjectionContext} from '@angular/core';
 import {toSignal} from '@angular/core/rxjs-interop';
+import {TestBed} from '@angular/core/testing';
 import {BehaviorSubject, ReplaySubject, Subject} from 'rxjs';
 
 describe('toSignal()', () => {
@@ -109,17 +110,16 @@ describe('toSignal()', () => {
     expect(counter()).toBe(1);
   });
 
-  it('should allow toSignal creation in a reactive context - issue 51027', () => {
+  it('should not allow toSignal creation in a reactive context', () => {
     const counter$ = new BehaviorSubject(1);
-
-    const injector = Injector.create([]);
-
     const doubleCounter = computed(() => {
-      const counter = toSignal(counter$, {requireSync: true, injector});
+      const counter = toSignal(counter$, {requireSync: true});
       return counter() * 2;
     });
 
-    expect(doubleCounter()).toBe(2);
+    expect(() => doubleCounter())
+        .toThrowError(
+            /toSignal\(\) cannot be called from within a reactive context. Invoking `toSignal` causes new subscriptions every time./);
   });
 
   describe('with no initial value', () => {
@@ -173,6 +173,31 @@ describe('toSignal()', () => {
          expect(counter()).not.toBeNull();
        }));
   });
+
+  describe('in a @Component', () => {
+    it('should support `toSignal` as a class member initializer', () => {
+      @Component({
+        template: '{{counter()}}',
+        changeDetection: ChangeDetectionStrategy.OnPush,
+      })
+      class TestCmp {
+        // Component creation should not run inside the template effect/consumer,
+        // hence using `toSignal` should be allowed/supported.
+        counter$ = new Subject<number>();
+        counter = toSignal(this.counter$);
+      }
+
+      const fixture = TestBed.createComponent(TestCmp);
+      fixture.detectChanges();
+
+      expect(fixture.nativeElement.textContent).toBe('');
+
+      fixture.componentInstance.counter$.next(2);
+      fixture.detectChanges();
+
+      expect(fixture.nativeElement.textContent).toBe('2');
+    });
+  });
 });
 
 function test(fn: () => void|Promise<void>): () => Promise<void> {

packages/core/src/core_reactivity_export_internal.ts

@@ -27,4 +27,7 @@ export {
   ZoneAwareQueueingScheduler as ɵZoneAwareQueueingScheduler,
   FlushableEffectRunner as ɵFlushableEffectRunner,
 } from './render3/reactivity/effect';
+export {
+  assertNotInReactiveContext,
+} from './render3/reactivity/asserts';
 // clang-format on

packages/core/src/errors.ts

@@ -83,6 +83,7 @@ export const enum RuntimeErrorCode {
   // Signal Errors
   SIGNAL_WRITE_FROM_ILLEGAL_CONTEXT = 600,
   REQUIRE_SYNC_WITHOUT_SYNC_EMIT = 601,
+  ASSERTION_NOT_INSIDE_REACTIVE_CONTEXT = 602,
 
   // Styling Errors
 

packages/core/src/render3/reactivity/asserts.ts

@@ -0,0 +1,30 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {RuntimeError, RuntimeErrorCode} from '../../errors';
+import {getActiveConsumer} from '../../signals';
+
+/**
+ * Asserts that the current stack frame is not within a reactive context. Useful
+ * to disallow certain code from running inside a reactive context (see {@link toSignal}).
+ *
+ * @param debugFn a reference to the function making the assertion (used for the error message).
+ *
+ * @publicApi
+ */
+export function assertNotInReactiveContext(debugFn: Function, extraContext?: string): void {
+  // Taking a `Function` instead of a string name here prevents the unminified name of the function
+  // from being retained in the bundle regardless of minification.
+  if (getActiveConsumer() !== null) {
+    throw new RuntimeError(
+        RuntimeErrorCode.ASSERTION_NOT_INSIDE_REACTIVE_CONTEXT,
+        ngDevMode &&
+            `${debugFn.name}() cannot be called from within a reactive context.${
+                extraContext ? ` ${extraContext}` : ''}`);
+  }
+}

packages/core/src/signals/index.ts

@@ -9,7 +9,7 @@
 export {defaultEquals, isSignal, Signal, SIGNAL, ValueEqualityFn} from './src/api';
 export {computed, CreateComputedOptions} from './src/computed';
 export {setThrowInvalidWriteToSignalError} from './src/errors';
-export {consumerAfterComputation, consumerBeforeComputation, consumerDestroy, isInNotificationPhase, producerAccessed, producerNotifyConsumers, producerUpdatesAllowed, producerUpdateValueVersion, REACTIVE_NODE, ReactiveNode, setActiveConsumer} from './src/graph';
+export {consumerAfterComputation, consumerBeforeComputation, consumerDestroy, getActiveConsumer, isInNotificationPhase, producerAccessed, producerNotifyConsumers, producerUpdatesAllowed, producerUpdateValueVersion, REACTIVE_NODE, ReactiveNode, setActiveConsumer} from './src/graph';
 export {CreateSignalOptions, setPostSignalSetFn, signal, WritableSignal} from './src/signal';
 export {untracked} from './src/untracked';
 export {Watch, watch, WatchCleanupFn, WatchCleanupRegisterFn} from './src/watch';

packages/core/src/signals/src/graph.ts

@@ -26,6 +26,10 @@ export function setActiveConsumer(consumer: ReactiveNode|null): ReactiveNode|nul
   return prev;
 }
 
+export function getActiveConsumer(): ReactiveNode|null {
+  return activeConsumer;
+}
+
 export function isInNotificationPhase(): boolean {
   return inNotificationPhase;
 }