From 656bc282e345c5e37a9189a0a4daa631e02c31bf Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 25 Jan 2024 12:16:43 +0100 Subject: [PATCH] fix(core): add toString implementation to signals (#54002) Since signals are function, currently stringifying them reveals the implementation of the function. This can lead to confusion since it contains internal implementation details. These changes add static `toString` function to address the issue. **Note:** it's tempting to have `toString` output the actual value of the signal, but that would encourage users not to call the function which will be problematic in the long run. That's why these changes are using a static string instead. PR Close #54002 --- packages/core/primitives/signals/src/computed.ts | 4 ++++ packages/core/primitives/signals/src/signal.ts | 4 ++++ packages/core/src/authoring/input_signal.ts | 4 ++++ packages/core/src/render3/query_reactive.ts | 4 ++++ packages/core/test/authoring/input_signal_spec.ts | 5 +++++ packages/core/test/signals/computed_spec.ts | 6 ++++++ packages/core/test/signals/signal_spec.ts | 5 +++++ 7 files changed, 32 insertions(+) diff --git a/packages/core/primitives/signals/src/computed.ts b/packages/core/primitives/signals/src/computed.ts index 920f71ed2cc0c..c7b3fb3576a2a 100644 --- a/packages/core/primitives/signals/src/computed.ts +++ b/packages/core/primitives/signals/src/computed.ts @@ -40,6 +40,9 @@ export type ComputedGetter = (() => T)&{ [SIGNAL]: ComputedNode; }; +/** Function used as the `toString` implementation of computed. */ +const computedToString = () => '[COMPUTED]'; + /** * Create a computed signal which derives a reactive value from an expression. */ @@ -61,6 +64,7 @@ export function createComputed(computation: () => T): ComputedGetter { return node.value; }; (computed as ComputedGetter)[SIGNAL] = node; + computed.toString = computedToString; return computed as unknown as ComputedGetter; } diff --git a/packages/core/primitives/signals/src/signal.ts b/packages/core/primitives/signals/src/signal.ts index e30c3e6ce01a8..b512fc188823d 100644 --- a/packages/core/primitives/signals/src/signal.ts +++ b/packages/core/primitives/signals/src/signal.ts @@ -35,6 +35,9 @@ export interface SignalGetter extends SignalBaseGetter { readonly[SIGNAL]: SignalNode; } +/** Function used as the `toString` implementation of signals. */ +const signalToString = () => '[SIGNAL]'; + /** * Create a `Signal` that can be set or updated directly. */ @@ -46,6 +49,7 @@ export function createSignal(initialValue: T): SignalGetter { return node.value; }) as SignalGetter; (getter as any)[SIGNAL] = node; + getter.toString = signalToString; return getter; } diff --git a/packages/core/src/authoring/input_signal.ts b/packages/core/src/authoring/input_signal.ts index a9e8e19f1b538..672675027e5c1 100644 --- a/packages/core/src/authoring/input_signal.ts +++ b/packages/core/src/authoring/input_signal.ts @@ -68,6 +68,9 @@ export interface InputSignal extends Signal { [ɵINPUT_SIGNAL_BRAND_WRITE_TYPE]: WriteT; } +/** Function used as the `toString` implementation of input signals. */ +const signalInputToString = () => '[INPUT_SIGNAL]'; + /** * Creates an input signal. * @@ -99,5 +102,6 @@ export function createInputSignal( } (inputValueFn as any)[SIGNAL] = node; + inputValueFn.toString = signalInputToString; return inputValueFn as InputSignal; } diff --git a/packages/core/src/render3/query_reactive.ts b/packages/core/src/render3/query_reactive.ts index 8eac43e157bb7..8d8462689420b 100644 --- a/packages/core/src/render3/query_reactive.ts +++ b/packages/core/src/render3/query_reactive.ts @@ -18,6 +18,9 @@ import {collectQueryResults, getTQuery, loadQueryInternal, materializeViewResult import {Signal} from './reactivity/api'; import {getLView} from './state'; +/** Function used as the `toString` implementation of query signals. */ +const querySignalToString = () => '[QUERY_SIGNAL]'; + function createQuerySignalFn(firstOnly: true, required: true): Signal; function createQuerySignalFn(firstOnly: true, required: false): Signal; function createQuerySignalFn(firstOnly: false, required: false): Signal>; @@ -45,6 +48,7 @@ function createQuerySignalFn(firstOnly: boolean, required: boolean) { } } (signalFn as any)[SIGNAL] = node; + signalFn.toString = querySignalToString; return signalFn; } diff --git a/packages/core/test/authoring/input_signal_spec.ts b/packages/core/test/authoring/input_signal_spec.ts index 161adb365d806..83c65353cae3b 100644 --- a/packages/core/test/authoring/input_signal_spec.ts +++ b/packages/core/test/authoring/input_signal_spec.ts @@ -79,4 +79,9 @@ describe('input signal', () => { node.applyValueToInputSignal(node, 1); expect(expr()).toBe(1001); }); + + it('should have a toString implementation', () => { + const signal = input(0); + expect(signal + '').toBe('[INPUT_SIGNAL]'); + }); }); diff --git a/packages/core/test/signals/computed_spec.ts b/packages/core/test/signals/computed_spec.ts index 6d64c9c7f8099..1c645cfcc3657 100644 --- a/packages/core/test/signals/computed_spec.ts +++ b/packages/core/test/signals/computed_spec.ts @@ -186,4 +186,10 @@ describe('computed', () => { expect(illegal).toThrow(); }); + + it('should have a toString implementation', () => { + const counter = signal(0); + const double = computed(() => counter() * 2); + expect(double + '').toBe('[COMPUTED]'); + }); }); diff --git a/packages/core/test/signals/signal_spec.ts b/packages/core/test/signals/signal_spec.ts index c8da6a79ee862..8d9a127ec69d5 100644 --- a/packages/core/test/signals/signal_spec.ts +++ b/packages/core/test/signals/signal_spec.ts @@ -123,6 +123,11 @@ describe('signals', () => { expect(double()).toBe(4); }); + it('should have a toString implementation', () => { + const state = signal(false); + expect(state + '').toBe('[SIGNAL]'); + }); + describe('optimizations', () => { it('should not repeatedly poll status of a non-live node if no signals have changed', () => { const unrelated = signal(0);