platform icon indicating copy to clipboard operation
platform copied to clipboard

RFC(`@ngrx/signals`): Add `unprotected` helper for testing

Open markostanimirovic opened this issue 1 year ago • 5 comments

Which @ngrx/* package(s) are relevant/related to the feature request?

signals

Information

This RFC suggests introducing the unprotected function that would be exported from @ngrx/signals/testing. It would allow updating protected SignalStore's state in tests as follows:

// users.store.ts
const UsersStore = signalStore(withEntities<User>());

// users.store.spec.ts
import { unprotected } from '@ngrx/signals/testing';

const store = TestBed.inject(UsersStore);
patchState(unprotected(store), setAllEntities(usersMock));

Describe any alternatives/workarounds you're currently using

No response

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

  • [X] Yes
  • [ ] No

markostanimirovic avatar Sep 28 '24 10:09 markostanimirovic

We could also provide a function overrideState or overrideStore similar to the overrideXY methods in Angular's TestBed.

This would have two advantages:

✅ Similarity to what people know from Angular ✅ It does not "invite" people to break up coupling for testing too much.

Ofc, breaking up coupling is also possible with overrideXY, but IMHO it guides people more into the direction of setting up an object under test by mocking some aspect and then running a test against the public API.

manfredsteyer avatar Sep 29 '24 13:09 manfredsteyer

Yeah, good point but then I'd suggest to go with something like

const store = injectUnprotectedStore(UsersStore);
patchState(store, setAllEntities(usersMock))

There is a perceived difference between the override functions of the TestBed. Whereas with override we still get the same type, with injectUnprotectedStore or unprotected, we'll get another type.


In order to avoid misuse, we could say, the only need for the test helper is to setup the initial state...and nothing else. In that case, we could have just injectStore with a second Parameter, which is the same as of patchState

const store = injectStore(UsersStore, setAllEntities(usersMock);

In this case, the state stays protected for the test as well.

For edge cases we could still have unprotected, but I would promote more the injectStore.

rainerhahnekamp avatar Sep 29 '24 13:09 rainerhahnekamp

I think injectUnprotectedStore results in fewer mistakes. With unprotected, there's a possibility of unintended state changes occurring.

mzkmnk avatar Oct 03 '24 12:10 mzkmnk

I think injectUnprotectedStore results in fewer mistakes. With unprotected, there's a possibility of unintended state changes occurring.

What do you mean by unintended state changes? The unprotected function would be exposed by the testing plugin, not the core package.

markostanimirovic avatar Oct 07 '24 20:10 markostanimirovic

What do you mean by unintended state changes? The unprotected function would be exposed by the testing plugin, not the core package.

I see, there doesn’t seem to be a problem if it’s a testing plugin.

mzkmnk avatar Oct 07 '24 23:10 mzkmnk