[signals] Potential typing bug when using discriminated unions

[jits]

Which @ngrx/* package(s) are the source of the bug?

signals

Minimal reproduction of the bug/regression with instructions

See https://stackblitz.com/edit/stackblitz-starters-ucjue4?file=src%2Fstore.ts

For posterity, here is the code that causes the typing error:

import { Injectable } from '@angular/core';
import { signalStore, withState } from '@ngrx/signals';

type A = {
  status: 'A';
  thing: null;
};

type B = {
  status: 'B';
  thing: string;
};

type State = A | B;

const initialState: State = {
  status: 'A',
  thing: null,
};

const _Store = signalStore(
  { providedIn: 'root' },
  withState<State>(initialState)
);

@Injectable({ providedIn: 'root' })
export class Store extends _Store {}    // <-- Typing error occurs here

The typing error occurs on the line export class Store extends _Store {}, with the following TypeScript error:

Base constructor return type '({ status: Signal<"A">; thing: Signal<null>; } | { status: Signal<"B">; thing: Signal<string>; }) & StateSource<{ status: "A"; thing: null; } | { status: "B"; thing: string; }>' is not an object type or intersection of object types with statically known members.(2509)

Expected behavior

Should compile fine and not give any TypeScript errors.

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s)

NgRx: v18.0.0 Angular: latest v18 TypeScript: v5.5

Other information

No response

I would be willing to submit a PR to fix this issue

• [X] Yes
• [ ] No

[rainerhahnekamp]

The error goes away if you just have

const _Store = signalStore(
  { providedIn: 'root' },
  withState(initialState)
);

initialState has already a declared type, so no need to provide the type a second time. Not sure if that helps in the long run, though.

[jits]

Thanks @rainerhahnekamp! That works for now (as a workaround) 👍🏽

[rainerhahnekamp]

@markostanimirovic, if that's the right approach for now, should we leave a note in the docs?

[jits]

@rainerhahnekamp — I spoke too soon — unfortunately it still fails elsewhere, when using the discriminated union types. I've updated the Stackblitz to show this (https://stackblitz.com/edit/stackblitz-starters-ucjue4?file=src%2Fstore.ts).

Looks like the inferred state type of the store is not picking up the full discriminated union, just the specific type of initialState.

[markostanimirovic]

Union types can be used for state properties, not for the entire state because SignalStore/SignalState generates state signals based on the initial state.

// From:
type State = A | B;
const initialState: State = { /* ... */ };

// To:
type State = { prop: A | B };
const initialState: State = { prop: /* ... */ };

We can add a section about unions to the FAQ page and update the answer for the "Can I define my SignalStore as a class?" question to contain a note that although it's possible to create a class-based SignalStore, using the functional style is recommended.

[jits]

@markostanimirovic — ahh, that's a shame! I've been using discriminated union types (with extended class) up until (and including) RC2. Is there anything that can be done to support this use case, as I feel it's more akin to modelling state like a state machine, and gives us much better type safety. I'm keen to use this with extended classes so I can add extra properties etc. (as mentioned in https://github.com/ngrx/platform/discussions/4141#discussioncomment-7629034).

A concrete example of where I'm doing this (an AuthStore):

• Types
• Internal updater methods
• Extended class

[rainerhahnekamp]

@jits. nested state with union works. We can take that as workaround

type A = {
  status: 'A';
  thing: null;
};

type B = {
  status: 'B';
  thing: string;
};

type NestedState = A | B;

type State = { nested: NestedState };

const initialState: State = { nested: { status: 'A', thing: null } };

const _Store = signalStore(
  { providedIn: 'root' },
  withState(initialState),
  withMethods((store) => {
    return {
      updateToB(thing: string): void {
        const newState: B = { status: 'B', thing };
        patchState(store, { nested: newState });
      },
    };
  })
);

[markostanimirovic]

@markostanimirovic — ahh, that's a shame! I've been using discriminated union types (with extended class) up until (and including) RC2. Is there anything that can be done to support this use case, as I feel it's more akin to modelling state like a state machine, and gives us much better type safety. I'm keen to use this with extended classes so I can add extra properties etc. (as mentioned in #4141 (reply in thread)).

A concrete example of where I'm doing this (an AuthStore):

• Types
• Internal updater methods
• Extended class

Thank you for the detailed explanation @jits!

We'll consider adding a withProps base feature to add properties to the SignalStore in the next major release. With that feature, I guess class-based stores won't be needed anymore.

In the meantime, I suggest a different approach. Instead of extending a SignalStore, you can create a class that injects it:

const AuthStore = signalStore({ providedIn: 'root' }, /* ... */);

@Injectable({ providedIn: 'root' })
export class AuthFacade {
  readonly #store = inject(AuthStore);

 // ... expose all public store APIs
 // ... add additional properties
}

By the way, discriminated unions for the entire state work with functional SignalStores: https://stackblitz.com/edit/stackblitz-starters-hefo5v?file=src%2Fstore.ts

[jits]

Thanks for clarifying @markostanimirovic, and for your suggestions.

Sounds like functional SignalStores are the way forward. A future withProps base feature would be handy, but a wrapper / helper class will work for now.

Congrats to you and the team on the NgRx SignalStore first stable release! 🎉

(Please feel free to close this issue if no further action is needed.)