ngrx / platform

Reactive State for Angular
https://ngrx.io
Other
7.96k stars 1.95k forks source link

signalState changes passed array to map #4316

Closed dyeske61283 closed 1 month ago

dyeske61283 commented 2 months ago

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

signals

Minimal reproduction of the bug/regression with instructions

https://stackblitz.com/edit/angular-17-starter-project-cexglh?file=src%2Fmain.ts

Expected behavior

The type of the passed state should not change - even if no deep signals are created. (Looking at the unit test here: https://github.com/ngrx/platform/blob/51034e64833179e21039601875e4297433d870d5/modules/signals/spec/types/signal-state.types.spec.ts#L121)

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

@ngrx/signals: 17.2.0 @angular: 17.0.4

Other information

No response

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

markostanimirovic commented 2 months ago

signalState as well as withState should accept a record/dictionary as an input argument. There is no benefit of using them with arrays. at the root level.

There are two options that you can use:

1) const numbers = signal([1, 2, 3]); 2) const state = signalState({ numbers: [1, 2, 3] });

dyeske61283 commented 2 months ago

@markostanimirovic we are using the second option now, but some level of error or warning would have been nice in the development or at least in the documentation.

dreamstar-enterprises commented 2 months ago

Will deep signals be supported for maps like this? E.g. so computed signals can be created on specific keys, whose values might be arrays?

type FilterModalState = {
  filterModalMap: {
    [modalType: string]: {
      modalType: string;
      selectedItemsState: string[];
      cancelOrResetLabel: string;
    }
  }
};

Currently, I think only this works

type FilterModalState = {
  filterModalMap: {
    modalData: {
      modalType: string;
      selectedItemsState: string[];
      cancelOrResetLabel: string;
    }
  }
};