New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(signals): Encapsulation #4210
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for ngrx-io canceled.Built without sensitive environment variables
|
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. |
Thanks @jits. Coming from the global store, I would have Looking at it from the perspective of a locally provided store, it might be different, though. With |
@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 In terms of the breaking change, given the signal store is in "developer preview" now would be the time to make such a change 😄 |
@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. |
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:
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. Typing is straightforward, but requests to explicitly provide it (e.g. So we are discussing to do it through conventions right now, e.g. anything that has a |
@alex-okrushko, thanks for the update. We (@mikezks, @manfredsteyer) think the main requirement is to be able to prevent
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. |
@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 :
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
|
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
This is a prototype of a
signalStore
version which supports encapsulationvia 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 forpatchState
to{}
. Therefore completely disabling thepossibility to update the state from the outside.
readonlyExcept: Array<keyof Store>
: Selectively mark properties "unpatchable" forpatchState
.Example with
readonly
.Mor examples available at https://stackblitz.com/edit/stackblitz-starters-yb3emy?file=src%2Fmain.ts
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:
SignalStoreConfig
needs to be extendedsignalStore
needs to use the new typesEncapsulatedStore
and
EncapsulatedState
.easier - still has to be done.
signalStoreFeature
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?
Other information