feat(signals): enable creation of dynamic deep signals

[markostanimirovic]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/ngrx/platform/blob/main/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Documentation has been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Closes #4847

Fixes signals tests in #5173

What is the new behavior?

Enables creation of dynamic deep signals in signalState, signalStore, and deepComputed. Example:

type Book = { id: number; title: string };
type Status =
  | { type: 'success'; data: string }
  | { type: 'error'; message: string };

const BookStore = signalStore(
  withState<{ book: Book | null; status: Status }>({
    book: null,
    status: { type: 'success', data: '' },
  })
);
const store = inject(BookStore);

// 👇 object literal + null: store.book is DeepSignal<Book> | Signal<null>
if ('title' in store.book) {
  const title = store.book.title; // Signal<string>
  console.log(title());
}

// 👇 union of object literals: a DeepSignal is created for each member
// store.status: DeepSignal<{ type: 'success'; data: string }> | DeepSignal<{ type: 'error'; message: string }>
if ('message' in store.status) {
  const message = store.status.message; // Signal<string>
  console.log(message());
}

Does this PR introduce a breaking change?

[x] Yes
[ ] No

BREAKING CHANGES:

Union state slices and computed results that include an object literal now create a `DeepSignal` for each object literal member, instead of exposing the whole union as a single `Signal`.

BEFORE:

A union that included an object literal was exposed as a single `Signal` of the whole union.

signalState:

const state = signalState<{ user: { name: string } | null }>({
  user: null
});
// state.user: Signal<{ name: string } | null>

signalStore:

const Store = signalStore(
  withState<{ user: { name: string } | null }>({ user: null })
);
const store = inject(Store);
// store.user: Signal<{ name: string } | null>

deepComputed:

const source = signal<{ a: number } | { b: number }>({ a: 1 });
const result = deepComputed(() => source());
// result: Signal<{ a: number } | { b: number }>

Custom SignalStore feature with generics:

function withMyFeature<Entity extends { id: number }>() {
  return signalStoreFeature(
    type<{ state: { entity: Entity | null } }>(),
    withMethods(({ entity }) => {
      // the type of entity is Signal<Entity | null>
      const e: Signal<Entity | null> = entity;

      return {
        // ...
      };
    })
  );
}

AFTER:

Each object literal member becomes its own `DeepSignal`; the remaining members stay a regular `Signal`.

signalState:

const state = signalState<{ user: { name: string } | null }>({
  user: null
});
// state.user: DeepSignal<{ name: string }> | Signal<null>

signalStore:

const Store = signalStore(
  withState<{ user: { name: string } | null }>({ user: null })
);
const store = inject(Store);
// store.user: DeepSignal<{ name: string }> | Signal<null>

deepComputed:

const source = signal<{ a: number } | { b: number }>({ a: 1 });
const result = deepComputed(() => source());
// result: DeepSignal<{ a: number }> | DeepSignal<{ b: number }>

Custom SignalStore feature with generics:

function withMyFeature<Entity extends { id: number }>() {
  return signalStoreFeature(
    type<{ state: { entity: Entity | null } }>(),
    withMethods(({ entity }) => {
      // the type of entity is DeepSignalOf<Entity | null>
      const e: DeepSignalOf<Entity | null> = entity;

      return {
        // ...
      };
    })
  );
}

[rainerhahnekamp]

As always, great stuff! I just wanted to bring up two points for discussion:

1. Typing Issues with Deep Signals

This isn't necessarily a new issue, but it became much clearer to me during this review. The following snippet shows how our deep signal feature can lead to some tricky type safety gaps:

it('gives a wrong type guarantee', () => {
  type Status =
    | { status: 'error'; message: string }
    | { status: 'success'; value: number };
  
  const state = signal({
    status: 'error',
    message: 'did not work',
  } as Status);

  const deepState = toDeepSignal(state);
  
  if ('message' in deepState) {
    const message = deepState.message;
    state.set({ status: 'success', value: 1 });

    // returns undefined although it is typed as Signal<string> 😬
    message().toUpperCase(); 
  }
});

While we could argue that developers shouldn't access a message signal without a proper runtime check, I still see this as a problem from a strict TypeScript perspective.

Strictly speaking, message should be typed as Signal<string | undefined>. However, I wouldn't want to make that change, as I'm really hoping the scenario above is just an edge case. Instead, I suggest we simply throw a runtime error.

---

2. Vitest Type Tests vs. ts-snippet

Regarding our testing setup, why don't we use Vitest's built-in type testing capabilities instead of ts-snippet? I found that they worked quite nicely for in PR #5186.

[markostanimirovic]

@rainerhahnekamp

1. Typing Issues with Deep Signals

This isn't necessarily a new issue, but it became much clearer to me during this review. The following snippet shows how our deep signal feature can lead to some tricky type safety gaps:

it('gives a wrong type guarantee', () => {
  type Status =
    | { status: 'error'; message: string }
    | { status: 'success'; value: number };
  
  const state = signal({
    status: 'error',
    message: 'did not work',
  } as Status);

  const deepState = toDeepSignal(state);
  
  if ('message' in deepState) {
    const message = deepState.message;
    state.set({ status: 'success', value: 1 });

    // returns undefined although it is typed as Signal<string> 😬
    message().toUpperCase(); 
  }
});

While we could argue that developers shouldn't access a message signal without a proper runtime check, I still see this as a problem from a strict TypeScript perspective.

Strictly speaking, message should be typed as Signal<string | undefined>. However, I wouldn't want to make that change, as I'm really hoping the scenario above is just an edge case. Instead, I suggest we simply throw a runtime error.

The same issue can be reproduced with plain TypeScript, and this is not something we can control:

    type Status = { status: 'success' } | { status: 'error'; message: string; }
    const state: Status = { status: 'error', message: 'x' };
    if (state.status === 'error') {
      Object.assign(state, { status: 'success', message: undefined });
      state.message.toUpperCase(); // typed string, undefined at runtime. Compiles clean.
    }

Anyway, I don't see this as any kind of blocker - in properly implemented logic, write and read blocks should be separated, and type narrowing of deep signals is needed only for the read logic. If someone wants to abuse our APIs, there is always a way (as any as a last resort 😅).

2. Vitest Type Tests vs. ts-snippet

Regarding our testing setup, why don't we use Vitest's built-in type testing capabilities instead of ts-snippet? I found that they worked quite nicely for in PR #5186.

For consistency reasons, I don't think we should mix both within a single project (@ngrx/signals in this case still relies on ts-snippet). I haven't created new spec files, only extended the existing ones. So the reason is simple - it's out of scope for this PR. Refactoring of @ngrx/signals type tests from ts-snippet tests to Vitest should be tackled separately.

[rainerhahnekamp]

As discussed, we see my first comment as a theoretical concern and wait if those things actually happen in real life.
As for the typing tests, they would be handled in a separate issue.

[timdeschryver]

LGTM, I also asked a review from codex, which gave 1 comment.