-
Notifications
You must be signed in to change notification settings - Fork 10
freeze state in signalState #2
base: main
Are you sure you want to change the base?
freeze state in signalState #2
Conversation
… a function, i.e. state is exposed. @markostanimirovic: I am not happy with the `@ts-ignore` in `deepFreeze`. Maybe you can find a better way.
src/lib/signal-state.ts
Outdated
| export function signalState<State extends Record<string, unknown>>( | ||
| initialState: State | ||
| ): SignalState<State> { | ||
| deepFreeze(initialState); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should perform deepFreeze only in dev mode for better performance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still working on that
| @@ -1,5 +1,4 @@ | |||
| import { | |||
| selectSignal, | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test1.ts and test2.ts files contain "type tests 😅", to make sure that typing works fine after refactoring. When we move this lib to the NgRx repo, they will be converted into real type tests. For now, they contain a few commented lines that use selectSignal so feel free to revert this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. I've left testEffects there as it is more a testing file and not a test
- rename withEffect to testEffects - type assertion in freezing function - move signal state spec back to original location
…ovides state as readonly - refactored tests - added further tests for mutable and immutable changes
|
Some comments from my latest push:
|
|
Have you tried to run the playground app? It seems that the If you agree, we can keep only the Regarding the if (isDevMode()) {
deepFreeze(state);
}Btw, |
|
Yeah, you're right. Let's get that freeze feature in and discuss the rest in a separate PR. |
It is currently possible to run mutable changes via the
$updatefunction. Those mutable changes do not throw any error, and effects tracking that part of the state are also not executed. The tests are located in the "immutability" tests suite of signal-state.spec.ts.This change applies
Object.freezeto the state, causing an error on mutable changes.The implementation is an almost identical copy of
freezein NgRx.Other than that,
withEffectas a test utility method to simplify the tests of effects.