Document usage of DefaultRouterStateSerializer with StoreDevTools

[dev054]

Once you use the DefaultRouterStateSerializer, it's impracticable to use StoreDevTools, as it freezes due to the Excessive use of memory and CPU.

References:
https://stackoverflow.com/questions/61594594/ngrx-and-redux-dev-tool-performace-issue-due-to-excessive-use-of-memory-and-cpu
zalmoxisus/redux-devtools-extension#455

Sample:

imports: [
  StoreRouterConnectingModule.forRoot({
    serializer: DefaultRouterStateSerializer,
  }),
],

I wanted to request the inclusion of some documentation/example on how to sanitize it to prevent StoreDevTools' freeze. Currently I don't know what's the best way for this or even if it's possible.

Other information:

I must use DefaultRouterStateSerializer as it's required for NX Data Persistence works properly.

[david-shortman]

A similar notice already exists here for immutability checks being broken https://ngrx.io/guide/store/configuration/runtime-checks.

If the DefaultRouterStateSerializer is likely to cause multiple problems, maybe it shouldn't be the default?

Are you confident that the DefaultRouterStateSerializer is the thing causing the dev tools performance problem in your app? I know I recently used it in my large enterprise application and did not experience a slowdown in the dev tools.

[dev054]

Hey @david-shortman,

A similar notice already exists here for immutability checks being broken https://ngrx.io/guide/store/configuration/runtime-checks.

Yes, I created #2814 for this and then I noticed the alert about it and ended up disabling the immutability checks.

If the DefaultRouterStateSerializer is likely to cause multiple problems, maybe it shouldn't be the default?

Yes, I also commented it about its naming in #2686.

Are you confident that the DefaultRouterStateSerializer is the thing causing the dev tools performance problem in your app? I know I recently used it in my large enterprise application and did not experience a slowdown in the dev tools.

Unfortunately yes, once you start navigating in a large app, it freezes because the DefaultRouterStateSerializer stores everything and StoreDevTools can't handle this large recursive data.

[timdeschryver]

Feel free to create a Pull Request that mentions this (or to add a devtools recipe to the docs).
You can use serialize to add a custom serialization, or you can use actionsBlocklist.

[david-shortman]

I'll add some docs on this since I recently touched adding a warning during dev runtime for this.

[david-shortman]

The existing docs already mention that the Store Devtools are likely to suffer with the default serializer, offer to use the MinimalRouterStateSerializer, and explain how to implement a custom serializer.

I don't think additional documentation is necessary, although feedback on the clarity/organization of the existing docs would be helpful.

As a next step, since the DefaultRouterStateSerializer breaks the immutability principle, I think it would be useful to deprecate it and remove it, or make the MinimalRouterStateSerializer the default, and rename the default to something like FullRouterStateSerializer.

@timdeschryver Do you know the motivation to have the DefaultRouterStateSerializer as the default, and if either option above would be viable to helping devs avoid unnecessary data serialization?

[timdeschryver]

The runtime checks were added after the original router-store implementation.
With the added runtime checks, we had to come up with a solution to serialize the router state, and so came MinimalRouterStateSerializer to live. And, if I remember correctly, in earlier versions of Angular this wasn't a concern, but with a change (Ivy ?) the router's state became mutable (I could be wrong here). Router state was also one of the reasons to disable the serialization runtime check by default.

We didn't know how this would evolve, so we left things as they were.
Because DefaultRouterStateSerializer has been part of NgRx for a long time, I don't know how breaking it would be to completely remove it.

I do agree that it would make sense to rename the two serializers to DefaultRouterStateSerializer and FullRouterStateSerializer. Since there's still time for the v14 release, we could discuss this and take it from there. E.g. deprecate it in v14, and replace it in v15.

[timdeschryver]

@david-shortman we discussed this, and we all agree that this rename makes sense.
If you want, feel free to open a PR.
We'll also provide a schematic to do the rename on ng-update.

[timdeschryver]

Closing this in favor of #3416