Platform: Add Mutation and Serialization Checks into Store

[MikeRyanDev]

If we choose to implement #856 we should consider adding mutation checks directly into store. Think ngrx-state-freeze but built-in and it also checks if state can be serialized.

Considerations:

• State should only be checked when Angular is in development mode
• Checking should be the default but it should be able to be turned off when configuring the Store
• @ngrx/router-store should ship with a custom serializer turned on by default that allows the state to be frozen and serialized

[kylecordes]

Here is a potentially useful addition to the idea. Perhaps in this default development mode, in addition to freezing the objects, it could round-trip them through JSON at each pass through Store. This would expose problems with non-serializable data much earlier in the application development lifecycle, which I think would help some developers quite a lot.

I've seen various people/teams, both online and in our consulting and training work, where they build machinery around Store that both relies on mutable data and relies on data that cannot be serialized... and then realize that they either have to give up forever on the things that those limitations would enable, or rethink work that they already worked quite hard on.

[MikeRyanDev]

@kylecordes I love it.

[shyamal890]

+1

[tomwanzek]

@kylecordes I'm not sure, I fully follow the

Perhaps in this default development mode, in addition to freezing the objects, it could round-trip them through JSON at each pass through Store.

How would you suggest round-tripping handle Date objects upon de-serialization for general state data structures? class-transformer/class-validator based on decorated state classes as one might do on server-inbound json?

If anything, IMHO, such round-trip should be optional even in dev-mode and maybe not enabled by default. I have some fairly complex and length states, which I may not want to send through a spin cycle every time 😄

[kylecordes]

@tomwanzek Yes, exactly! By round tripping the data through JSON, developers would have to make a choice:

• Explicitly turn off this round-trip, explicitly giving up one of the advantages of this architecture (serializable state)
• implement custom serialization

I certainly don't suggest that in production the data go through such a thing, as it is pointlessly slow and wastes memory. But I am urging this to be the default during initial development, because we have tried the opposite default:

Up through now, the default has been no default tooling or settings which would guide developers to use serializable, immutable state. And what I see out there in the wild, is that most projects that have picked up Store (or the Redux architecture in general) do not have these desirable attributes. By the time folks understand the architecture enough to want those things, they already have a bunch of legacy code written without that understanding.

[tomwanzek]

@kylecordes I guess, one of my main concerns is that the implementation of the underlying serialization API does not foreclose critical use cases.

I.e. in one of my critical use cases I have entities, which have an outer skeleton structure which is known at design time, but the schemata which govern some of their nested properties, are only known at run time. These schemata themselves are entities of the store in their own right.

As ensuring that ngrx serialization does not shut out such a use case, I will add a slightly less abstract comment to the related issue #856.

[timdeschryver]

Is this up for grabs?
If so I'd like to implement/help to implement it (after the release of NgRx 6).

[MikeRyanDev]

@tdeschryver I have a prototype implementation: https://github.com/ngrx/platform/tree/feat/serialization-and-mutation-checks

I realized that we will need a few enhancements made to Store before we can land this:

• [ ] TypeScript 2.8 upgrade. Need this to produce a solid Immutable<T> type.
• [ ] API to allow library authors to provide their own meta reducers
• [ ] Mechanism to capture different runtime configuration options and assert if they are enabled or not

All three of these are prototyped in that branch. If you want to help carry it over the finish line you are more than welcome to do so.

[MikeRyanDev]

I should also point out that the API prototyped in the branch looks like this:

StoreModule.forRoot(reducers, {
  dangerouslyDisableRuntimeChecks: [
    RuntimeChecks.Immutability,
    RuntimeChecks.Serializability,
  ],
}),

but after discussing it with @brandonroberts we decided to go with a more TypeScript-inspired API:

StoreModule.forRoot(reducers, {
  runtimeOptions: {
    strictImmutabilityChecks: false,
    strictSerializabilityChecks: false,
  }
}),

[timdeschryver]

I gave it a quick glance and its looking good already :ok_hand:
I'm also in favor of the TS API.

Maybe some food for thought (because I was fiddling with this feature to get more familiar with the code base and these are the main differences):

• would an extra check on the serializability of an action before dispatching it add some value?
• instead of meta reducers, I implemented these checks in State.reduceState. I went for this approach because I was thinking these changes are part of the 'main library', and I see meta reducers as extensions.
• instead of using strictSerializabilityChecks, I extended StoreConfig with a serialize function which can be changed by the user's choice. But this was mainly because of the point mentioned above. That being said I think your way of doing it is a better one.

[jsonberry]

Just a note here to bring this up to the surface, the DataPersistence.navigation lib from @nrwl/nx does not play nice with the suggested custom serializer for the @ngrx/router-store, see https://github.com/nrwl/nx/issues/191#issuecomment-448312694

It's just important to keep custom serialization and store dev tool replacer configuration open for customization, but I don't think there's any talk about removing that 😄