feat(signals): Encapsulation

[rainerhahnekamp]

PR Checklist

Please check if your PR fulfills the following requirements:

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

PR Type

This is a prototype of a signalStore version which supports encapsulation via configuration.

Encapsulation only applies to the generated store, not to the features inside the store.

The extended configuration has the following properties:

• private: Array<keyof Store>: Encapsulates one or multiple property from the store.
• readonly: boolean: Sets the state for patchState to {}. Therefore completely disabling the possibility to update the state from the outside.
• readonlyExcept: Array<keyof Store>: Selectively mark properties "unpatchable" for patchState.

Example with readonly.

Mor examples available at https://stackblitz.com/edit/stackblitz-starters-yb3emy?file=src%2Fmain.ts

const Store = signalStore(
  {
    providedIn: 'root',
    readonly: true,
  },
  withState({
    id: 1,
    firstname: 'John',
    surname: 'List',
    birthday: new Date(1987, 5, 12),
  }),
  withComputed((state) => {
    return {
      prettyName: computed(() => `${state.firstname} ${state.surname}`),
      age: computed(
        () =>
          (new Date().getTime() - state.birthday().getTime()) /
          (1_000 * 60 * 60 * 24 * 365)
      ),
    };
  })
);

const store = new Store();
patchState(store, { id: 2 }); // causes a compilation error

This is a proof of concept for a signalStore version with a fixed amount of 3 features.

If this feature is considered to be added, following changes would be necessary:

1. SignalStoreConfig needs to be extended
2. The return type of signalStore needs to use the new types EncapsulatedStore and EncapsulatedState.
3. This version contains only the types. The actual implementation - which should be easier - still has to be done.
4. We might also extend the encapsulation to signalStoreFeature

[ ] 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?

The state can be updated via patchState from everywhere, which is not good. Especially for a global store.

Closes #

What is the new behavior?

We can encapsulate parts of the state, the complete state and specific methods, slices or signals.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

[netlify[bot]]

✅ Deploy Preview for ngrx-io canceled.

Built without sensitive environment variables

Name	Link
🔨 Latest commit	222f3d58f5c69d641f1df726edda4b6a933131d3
🔍 Latest deploy log	https://app.netlify.com/sites/ngrx-io/deploys/65a79fe627c1030008a01177

[jits]

This is a great idea @rainerhahnekamp.

Would it be too far-fetched to suggest that this should be the default behaviour? The fact that a store can be patched from anywhere it's accessible is a footgun, IMO.

[rainerhahnekamp]

Thanks @jits.

Coming from the global store, I would have readonly: true as default.

Looking at it from the perspective of a locally provided store, it might be different, though. With readonly: true, it would also be a (HUGE) breaking change.

[jits]

@rainerhahnekamp — makes sense.

I guess it comes down to what the default usage is expected to be; your config option could still be used to turn on patchState access outside of the store. I'm finding for the local stores I'm building I still want a strict interface to the store, so the component can't patch anything directly. (And I can still use signalState or plain signals in the component for other more direct access use cases [e.g. prototyping]).

In terms of the breaking change, given the signal store is in "developer preview" now would be the time to make such a change 😄

[rainerhahnekamp]

@markostanimirovic: Quick Follow up here. What should we do with that one? Should I create an accompanying issue where the community has the chance to discuss the API?

On the other hand, if you say, there's no chance this feature will land, we can close it as well.

[alex-okrushko]

Hi @rainerhahnekamp.

There's definitely a need for encapsulation, so that some props/methods can remain private.

There are a number of approaches that can do that:

1. Your current proposal, doing it through config
2. Creating custom store features
3. Enforce through typing
4. Introducing some conventions

The Drawbacks of (1) is that you'd have to declare the props/methods to include/exclude before they are even introduced.

Custom features might be a heavier-handed approach, (e.g. withExcludeProps('propA', 'methodB'))

Typing is straightforward, but requests to explicitly provide it (e.g. export type MyActualStore = Omit<Store, 'propA'|'methodB'>

So we are discussing to do it through conventions right now, e.g. anything that has a _ suffix will be removed from the type.

cc: @markostanimirovic

[rainerhahnekamp]

@alex-okrushko, thanks for the update.

We (@mikezks, @manfredsteyer) think the main requirement is to be able to prevent patchState from modifying a signalStore from the outside. That's what signalStore({readonly: true}) would do. As such, it's quite fitting as a config property.

readonlyExcept and private might be only for edge cases.

Please let me know if you come to a conclusion. We might integrate the encapsulation via a signalStoreFeature into our toolkit. That works without any changes from your side, and it might be a complementary feature to the convention approach you are discussing.

[gabrielguerrero]

@alex-okrushko @markostanimirovic , there is another way to support private props that you might want to consider, what if we support using symbols as props ?, similar to how we have STATE_SIGNAL, dev could create their own symbols for private props, something like :

const myPrivateProp = Symbol('myPrivateProp');
const myPrivateMethod = Symbol('myPrivateMethod');
const PrivateStore = signalStore(
  withState({ [myPrivateProp]: 'privateValue' }),
  withMethods(() => {
    return { [myPrivateMethod]: () => {} };
  }),
);

I think it will also require a few tweaks in the types, but will also need a small code change in the signalStore but I manage to have something that seems to work in the signal-store.ts file line 316 using an Object.assign instead of a for in will copy the symbols as well as string props

  @Injectable({ providedIn: config.providedIn || null })
  class SignalStore {
    constructor() {
      const innerStore = features.reduce(
        (store, feature) => feature(store),
        getInitialInnerStore()
      );
      const { slices, signals, methods, hooks } = innerStore;
      const props = { ...slices, ...signals, ...methods };

      (this as any)[STATE_SIGNAL] = innerStore[STATE_SIGNAL];

      // for (const key in props) {
      //   (this as any)[key] = props[key];
      // }
      Object.assign(this, props); // this will copy string and symbol keys, the for just copies sttring keys
      ``` 
   

[rainerhahnekamp]

It looks like this is becoming real. Further discussion and a new PR will be done here: #4405