From 7588948876beea61a437a7202236ff12558f9d40 Mon Sep 17 00:00:00 2001
From: markostanimirovic <markostanimirovic95@gmail.com>
Date: Tue, 3 Dec 2024 00:19:29 +0100
Subject: [PATCH] fix(signals): create deep signals for custom class instances

---
 modules/signals/spec/deep-signal.spec.ts      | 113 ++++++++++++++++++
 .../spec/types/signal-state.types.spec.ts     |  62 +++++++++-
 .../spec/types/signal-store.types.spec.ts     |  52 +++++++-
 modules/signals/src/deep-signal.ts            |  22 +++-
 modules/signals/src/ts-helpers.ts             |  15 +--
 5 files changed, 251 insertions(+), 13 deletions(-)
 create mode 100644 modules/signals/spec/deep-signal.spec.ts

diff --git a/modules/signals/spec/deep-signal.spec.ts b/modules/signals/spec/deep-signal.spec.ts
new file mode 100644
index 0000000000..0a0e1726d9
--- /dev/null
+++ b/modules/signals/spec/deep-signal.spec.ts
@@ -0,0 +1,113 @@
+import { toDeepSignal } from '../src/deep-signal';
+import { isSignal, signal } from '@angular/core';
+
+describe('toDeepSignal', () => {
+  it('creates deep signals for plain objects', () => {
+    const sig = signal({ m: { s: 't' } });
+    const deepSig = toDeepSignal(sig);
+
+    expect(sig).not.toBe(deepSig);
+
+    expect(isSignal(deepSig)).toBe(true);
+    expect(deepSig()).toEqual({ m: { s: 't' } });
+
+    expect(isSignal(deepSig.m)).toBe(true);
+    expect(deepSig.m()).toEqual({ s: 't' });
+
+    expect(isSignal(deepSig.m.s)).toBe(true);
+    expect(deepSig.m.s()).toBe('t');
+  });
+
+  it('creates deep signals for custom class instances', () => {
+    class User {
+      constructor(readonly firstName: string) {}
+    }
+
+    class UserState {
+      constructor(readonly user: User) {}
+    }
+
+    const sig = signal(new UserState(new User('John')));
+    const deepSig = toDeepSignal(sig);
+
+    expect(sig).not.toBe(deepSig);
+
+    expect(isSignal(deepSig)).toBe(true);
+    expect(deepSig()).toEqual({ user: { firstName: 'John' } });
+
+    expect(isSignal(deepSig.user)).toBe(true);
+    expect(deepSig.user()).toEqual({ firstName: 'John' });
+
+    expect(isSignal(deepSig.user.firstName)).toBe(true);
+    expect(deepSig.user.firstName()).toBe('John');
+  });
+
+  it('does not create deep signals for primitives', () => {
+    const num = signal(0);
+    const str = signal('str');
+    const bool = signal(true);
+
+    const deepNum = toDeepSignal(num);
+    const deepStr = toDeepSignal(str);
+    const deepBool = toDeepSignal(bool);
+
+    expect(deepNum).toBe(num);
+    expect(deepStr).toBe(str);
+    expect(deepBool).toBe(bool);
+  });
+
+  it('does not create deep signals for built-in object types', () => {
+    const array = signal([]);
+    const set = signal(new Set());
+    const map = signal(new Map());
+    const date = signal(new Date());
+    const error = signal(new Error());
+    const regExp = signal(new RegExp(''));
+
+    const deepArray = toDeepSignal(array);
+    const deepSet = toDeepSignal(set);
+    const deepMap = toDeepSignal(map);
+    const deepDate = toDeepSignal(date);
+    const deepError = toDeepSignal(error);
+    const deepRegExp = toDeepSignal(regExp);
+
+    expect(deepArray).toBe(array);
+    expect(deepSet).toBe(set);
+    expect(deepMap).toBe(map);
+    expect(deepDate).toBe(date);
+    expect(deepError).toBe(error);
+    expect(deepRegExp).toBe(regExp);
+  });
+
+  it('does not create deep signals for functions', () => {
+    const fn1 = signal(new Function());
+    const fn2 = signal(function () {});
+    const fn3 = signal(() => {});
+
+    const deepFn1 = toDeepSignal(fn1);
+    const deepFn2 = toDeepSignal(fn2);
+    const deepFn3 = toDeepSignal(fn3);
+
+    expect(deepFn1).toBe(fn1);
+    expect(deepFn2).toBe(fn2);
+    expect(deepFn3).toBe(fn3);
+  });
+
+  it('does not create deep signals for custom class instances that extend built-in object types', () => {
+    class CustomArray extends Array {}
+    class CustomSet extends Set {}
+    class CustomError extends Error {}
+
+    const array = signal(new CustomArray());
+    const set = signal(new CustomSet());
+    const error = signal(new CustomError());
+
+    const deepArray = toDeepSignal(array);
+    const deepSet = toDeepSignal(set);
+    const deepError = toDeepSignal(error);
+
+    expect(deepArray).toBe(array);
+    expect(deepSet).toBe(set);
+    expect(deepError).toBe(error);
+  });
+});
diff --git a/modules/signals/spec/types/signal-state.types.spec.ts b/modules/signals/spec/types/signal-state.types.spec.ts
index c9edeab141..d7fe53c47a 100644
--- a/modules/signals/spec/types/signal-state.types.spec.ts
+++ b/modules/signals/spec/types/signal-state.types.spec.ts
@@ -132,6 +132,20 @@ describe('signalState', () => {
     );
   });
 
+  it('does not create deep signals for Set', () => {
+    const snippet = `
+      const state = signalState(new Set<number>());
+      declare const stateKeys: keyof typeof state;
+    `;
+
+    expectSnippet(snippet).toSucceed();
+
+    expectSnippet(snippet).toInfer(
+      'stateKeys',
+      'unique symbol | keyof Signal<Set<number>>'
+    );
+  });
+
   it('does not create deep signals for Map', () => {
     const snippet = `
       const state = signalState(new Map<number, { bar: boolean }>());
@@ -146,9 +160,9 @@ describe('signalState', () => {
     );
   });
 
-  it('does not create deep signals for Set', () => {
+  it('does not create deep signals for Date', () => {
     const snippet = `
-      const state = signalState(new Set<number>());
+      const state = signalState(new Date());
       declare const stateKeys: keyof typeof state;
     `;
 
@@ -156,7 +170,49 @@ describe('signalState', () => {
 
     expectSnippet(snippet).toInfer(
       'stateKeys',
-      'unique symbol | keyof Signal<Set<number>>'
+      'unique symbol | keyof Signal<Date>'
+    );
+  });
+
+  it('does not create deep signals for Error', () => {
+    const snippet = `
+      const state = signalState(new Error());
+      declare const stateKeys: keyof typeof state;
+    `;
+
+    expectSnippet(snippet).toSucceed();
+
+    expectSnippet(snippet).toInfer(
+      'stateKeys',
+      'unique symbol | keyof Signal<Error>'
+    );
+  });
+
+  it('does not create deep signals for RegExp', () => {
+    const snippet = `
+      const state = signalState(new RegExp(''));
+      declare const stateKeys: keyof typeof state;
+    `;
+
+    expectSnippet(snippet).toSucceed();
+
+    expectSnippet(snippet).toInfer(
+      'stateKeys',
+      'unique symbol | keyof Signal<RegExp>'
+    );
+  });
+
+  it('does not create deep signals for Function', () => {
+    const snippet = `
+      const state = signalState(() => {});
+      declare const stateKeys: keyof typeof state;
+    `;
+
+    expectSnippet(snippet).toSucceed();
+
+    expectSnippet(snippet).toInfer(
+      'stateKeys',
+      'unique symbol | keyof Signal<() => void>'
     );
   });
 
diff --git a/modules/signals/spec/types/signal-store.types.spec.ts b/modules/signals/spec/types/signal-store.types.spec.ts
index aeb0ab73f4..85f2f5f5a2 100644
--- a/modules/signals/spec/types/signal-store.types.spec.ts
+++ b/modules/signals/spec/types/signal-store.types.spec.ts
@@ -175,6 +175,18 @@ describe('signalStore', () => {
     expectSnippet(snippet).toInfer('storeKeys', 'unique symbol');
   });
 
+  it('does not create deep signals when state type is Set', () => {
+    const snippet = `
+      const Store = signalStore(withState(new Set<{ foo: string }>()));
+      const store = new Store();
+      declare const storeKeys: keyof typeof store;
+    `;
+
+    expectSnippet(snippet).toSucceed();
+
+    expectSnippet(snippet).toInfer('storeKeys', 'unique symbol');
+  });
+
   it('does not create deep signals when state type is Map', () => {
     const snippet = `
       const Store = signalStore(withState(new Map<string, { foo: number }>()));
@@ -187,9 +199,45 @@ describe('signalStore', () => {
     expectSnippet(snippet).toInfer('storeKeys', 'unique symbol');
   });
 
-  it('does not create deep signals when state type is Set', () => {
+  it('does not create deep signals when state type is Date', () => {
     const snippet = `
-      const Store = signalStore(withState(new Set<{ foo: string }>()));
+      const Store = signalStore(withState(new Date()));
+      const store = new Store();
+      declare const storeKeys: keyof typeof store;
+    `;
+
+    expectSnippet(snippet).toSucceed();
+
+    expectSnippet(snippet).toInfer('storeKeys', 'unique symbol');
+  });
+
+  it('does not create deep signals when state type is Error', () => {
+    const snippet = `
+      const Store = signalStore(withState(new Error()));
+      const store = new Store();
+      declare const storeKeys: keyof typeof store;
+    `;
+
+    expectSnippet(snippet).toSucceed();
+
+    expectSnippet(snippet).toInfer('storeKeys', 'unique symbol');
+  });
+
+  it('does not create deep signals when state type is RegExp', () => {
+    const snippet = `
+      const Store = signalStore(withState(new RegExp('')));
+      const store = new Store();
+      declare const storeKeys: keyof typeof store;
+    `;
+
+    expectSnippet(snippet).toSucceed();
+
+    expectSnippet(snippet).toInfer('storeKeys', 'unique symbol');
+  });
+
+  it('does not create deep signals when state type is Function', () => {
+    const snippet = `
+      const Store = signalStore(withState(() => () => {}));
       const store = new Store();
       declare const storeKeys: keyof typeof store;
     `;
diff --git a/modules/signals/src/deep-signal.ts b/modules/signals/src/deep-signal.ts
index 733e085fc7..2342fd811a 100644
--- a/modules/signals/src/deep-signal.ts
+++ b/modules/signals/src/deep-signal.ts
@@ -47,5 +47,25 @@ export function toDeepSignal<T>(signal: Signal<T>): DeepSignal<T> {
 }
 
 function isRecord(value: unknown): value is Record<string, unknown> {
-  return value?.constructor === Object;
+  if (value === null || typeof value !== 'object') {
+    return false;
+  }
+
+  let proto = Object.getPrototypeOf(value);
+  if (proto === Object.prototype) {
+    return true;
+  }
+
+  while (proto && proto !== Object.prototype) {
+    if (
+      [Array, Set, Map, Date, Error, RegExp, Function].includes(
+        proto.constructor
+      )
+    ) {
+      return false;
+    }
+    proto = Object.getPrototypeOf(proto);
+  }
+
+  return proto === Object.prototype;
 }
diff --git a/modules/signals/src/ts-helpers.ts b/modules/signals/src/ts-helpers.ts
index f7df5cdb77..4117c1502e 100644
--- a/modules/signals/src/ts-helpers.ts
+++ b/modules/signals/src/ts-helpers.ts
@@ -1,13 +1,14 @@
 export type Prettify<T> = { [K in keyof T]: T[K] } & {};
 
 export type IsRecord<T> = T extends object
-  ? T extends unknown[]
-    ? false
-    : T extends Set<unknown>
-    ? false
-    : T extends Map<unknown, unknown>
-    ? false
-    : T extends Function
+  ? T extends
+      | unknown[]
+      | Set<unknown>
+      | Map<unknown, unknown>
+      | Date
+      | Error
+      | RegExp
+      | Function
     ? false
     : true
   : false;