feat(Store): runtime checks #1613

timdeschryver · 2019-03-09T16:17:53Z

PR Checklist

Please check if your PR fulfills the following requirements:

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

PR Type

What kind of change does this PR introduce?

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

Closes #857

What is the new behavior?

State serialization checks
Action serialization checks
Mutation checks
Remove ngrx-store-freeze ?
Remove non-serializable properties on the router ?

Does this PR introduce a breaking change?

[x] Yes
[ ] No

BREAKING CHANGE:

The META_REDUCERS token has been renamed to USER_PROVIDED_META_REDUCERS
The META_REDUCERS token has become a multi token and can be used by
library authors

Other information

Based of feat/serialization-and-mutation-checks

This PR also affects #1539

timdeschryver · 2019-03-09T16:25:39Z

modules/store/src/state.ts

+        this.next(state);
+        scannedActions.next(action);
+      },
+      error: err => this.error(err),


This change was needed in order for me to test the case with an invalid next state.

ngrxbot · 2019-03-09T16:32:03Z

Preview docs changes for f4b02ae at https://previews.ngrx.io/pr1613-f4b02ae/

ngrxbot · 2019-03-09T16:39:57Z

Preview docs changes for 0f52c54 at https://previews.ngrx.io/pr1613-0f52c54/

ngrxbot · 2019-03-09T18:39:47Z

Preview docs changes for 8e3eaf8 at https://previews.ngrx.io/pr1613-8e3eaf8/

ngrxbot · 2019-03-10T17:13:24Z

Preview docs changes for 38e9cc1 at https://previews.ngrx.io/pr1613-38e9cc1/

timdeschryver · 2019-03-11T19:28:18Z

Is this the API we had in mind?

brandonroberts · 2019-03-12T12:56:44Z

Yes, I like the API. A few thoughts:

I wonder how much router-store would be impacted from removing paramMap and queryParamMap?
I think this has the potential to uncover many accidental mutations in peoples apps (break them).
What's the migration strategy for those overriding the META_REDUCERS token today?

timdeschryver · 2019-03-12T18:04:30Z

I wonder how much router-store would be impacted from removing paramMap and queryParamMap?

We still have params and queryParams which is the same object (?) but without some getters or util functions.

I think this has the potential to uncover many accidental mutations in peoples apps (break them).

What about making these checks opt-in v8 and log a warning on start up (in dev) that these checks will be the default in v9 or v8.x?

What's the migration strategy for those overriding the META_REDUCERS token today?

Is it correct that if we would flatten out the meta reducers inside the concatMetaReducers function this shouldn't be a problem? The only issue would be that these reducers would be registered as an internal or library meta-reducer
To test this out I will create a test 😄

timdeschryver · 2019-03-12T18:24:19Z

See fcd21bb, I lied. The user also has to add multi: true.

But now that I'm thinking about it, the user has to change the token from META_REDUCERS to USER_PROVIDED_META_REDUCERS.

ngrxbot · 2019-03-12T18:29:09Z

Preview docs changes for fcd21bb at https://previews.ngrx.io/pr1613-fcd21bb/

timdeschryver · 2019-03-12T20:06:00Z

For an easy transition we could provide a linter and possibly add it to the migration script?
Something like https://github.com/timdeschryver/tslint-playground/blob/master/src/rules/noMetaReducersRule.ts.

ngrxbot · 2019-03-13T07:14:27Z

Preview docs changes for 89a9b21 at https://previews.ngrx.io/pr1613-89a9b21/

brandonroberts · 2019-03-14T19:59:31Z

I think we should do a separate PR to remove the non-serializable parts (paramMap, queryParamMap) from router-store so we aren't breaking too many things here.

brandonroberts · 2019-03-14T19:20:07Z

modules/store/src/index.ts

@@ -6,6 +6,7 @@ export {
  MetaReducer,
  Selector,
  SelectorWithProps,
+  RuntimeChecks,


These don't need to be exported publicly except for USER_PROVIDED_META_REDUCERS

brandonroberts · 2019-03-14T19:39:35Z

projects/example-app/src/app/app.module.ts

+      metaReducers,
+      runtimeChecks: {
+        strictStateSerializabilityChecks: false,
+        strictActionSerializabilityChecks: false,


Where are we mutating actions? We should clean this up.

ngrxbot · 2019-03-16T15:00:40Z

Preview docs changes for 60ce1c9 at https://previews.ngrx.io/pr1613-60ce1c9/

ngrxbot · 2019-03-16T17:15:56Z

Preview docs changes for 0bd736a at https://previews.ngrx.io/pr1613-0bd736a/

When #1613 gets merged we have mutation checks built in

ngrxbot · 2019-03-21T07:30:56Z

Preview docs changes for b553983 at https://previews.ngrx.io/pr1613-b553983/

brandonroberts · 2019-03-21T11:46:13Z

modules/store/src/models.ts

@@ -39,3 +39,9 @@ export type SelectorWithProps<State, Props, Result> = (
  state: State,
  props: Props
 ) => Result;
+
+export interface RuntimeChecks {
+  strictStateSerializabilityChecks: boolean;


We are already calling these RuntimeChecks, so we can remove Checks from the properties themselves.

brandonroberts · 2019-03-21T11:52:00Z

modules/router-store/spec/utils.ts

@@ -29,7 +29,14 @@ export function createTestModule(
  TestBed.configureTestingModule({
    declarations: [AppCmp, SimpleCmp],
    imports: [
-      StoreModule.forRoot(opts.reducers),
+      StoreModule.forRoot(opts.reducers, {


Remove this as its opt-in

brandonroberts · 2019-03-21T11:54:22Z

modules/store/src/runtime_checks.ts

+  if (isDevMode()) {
+    if (runtimeChecks === undefined) {
+      console.warn(
+        '@ngrx/store: we added immutability and serializability runtime checks in @ngrx/store.\n' +


I think we should keep this message short and add information about the checks to a page we link to. Another idea is to make this a comment for the RootStoreConfig so it shows up in their IDE and makes the console less noisy.

Yea, you're right 😅 the link to a page is a good idea 👍
My intention was to log it so it will certainly be visible, but this can quickly become annoying. It will also log out the messages in people's tests... On the other hand I'm afraid this change will be unnoticed by teams if don't log it and only add a comment on RootStoreConfig.

The consolw.warn can be "disabled" by providing a config, this can just be an empty object.

What do you think of logging:

@ngrx/store: runtime checks are currently opt-in but will be the default in the next major version, see -link- for more information.

The link can point to the changelog, or the migration page on ngrx.io

ngrxbot · 2019-03-21T18:55:33Z

Preview docs changes for 74b5ce7 at https://previews.ngrx.io/pr1613-74b5ce7/

ngrxbot · 2019-03-21T19:15:00Z

Preview docs changes for 9d66646 at https://previews.ngrx.io/pr1613-9d66646/

ngrxbot · 2019-03-24T20:47:31Z

Preview docs changes for 9242ce3 at https://previews.ngrx.io/pr1613-9242ce3/

ngrxbot · 2019-03-24T21:08:48Z

Preview docs changes for 9242ce3 at https://previews.ngrx.io/pr1613-9242ce3/

brandonroberts · 2019-03-31T19:58:54Z

@timdeschryver will you rebase so we can land this?

brandonroberts · 2019-03-31T19:59:31Z

Does this need to land as a rebase? Are there any breaking change commits? If so, the other commit messages need to be cleaned up.

This includes serialization checks and mutation checks BREAKING CHANGE: The META_REDUCERS token has been renamed to USER_PROVIDED_META_REDUCERS The META_REDUCERS token has become a multi token and can be used by library authors

timdeschryver · 2019-04-01T09:44:08Z

I've rebased and added a BREAKING CHANGE comment

ngrxbot · 2019-04-01T09:48:48Z

Preview docs changes for 218a1a4 at https://previews.ngrx.io/pr1613-218a1a4/

timdeschryver force-pushed the pr/runtime-checks branch from c2671f7 to f4b02ae Compare March 9, 2019 16:22

timdeschryver commented Mar 9, 2019

View reviewed changes

brandonroberts added the Breaking Change label Mar 14, 2019

brandonroberts requested changes Mar 14, 2019

View reviewed changes

timdeschryver marked this pull request as ready for review March 16, 2019 17:15

timdeschryver mentioned this pull request Mar 16, 2019

feat(RouterStore): remove unserializable properties #1630

Closed

3 tasks

timdeschryver added a commit that referenced this pull request Mar 17, 2019

refactor(Example): remove ngrx-store-freeze

cba6676

When #1613 gets merged we have mutation checks built in

This was referenced Mar 17, 2019

refactor(Example): remove ngrx-store-freeze #1633

Merged

feat(Store): add META_REDUCERS replacement migration #1640

Merged

feat(Store): define meta-reducers with a token #1539

Closed

brandonroberts requested changes Mar 21, 2019

View reviewed changes

timdeschryver force-pushed the pr/runtime-checks branch from 74b5ce7 to 9d66646 Compare March 21, 2019 19:06

brandonroberts approved these changes Mar 25, 2019

View reviewed changes

timdeschryver added 2 commits April 1, 2019 11:24

feat(Store): add runtime checks

265daea

This includes serialization checks and mutation checks BREAKING CHANGE: The META_REDUCERS token has been renamed to USER_PROVIDED_META_REDUCERS The META_REDUCERS token has become a multi token and can be used by library authors

feat(Store): rethrow state errors

218a1a4

timdeschryver force-pushed the pr/runtime-checks branch from 9242ce3 to 218a1a4 Compare April 1, 2019 09:42

brandonroberts merged commit 60633b7 into master Apr 1, 2019

brandonroberts deleted the pr/runtime-checks branch April 1, 2019 12:50

timdeschryver mentioned this pull request Apr 1, 2019

Example: enable immutability checks #1683

Closed

timdeschryver mentioned this pull request Apr 10, 2019

docs: Runtime checks #1726

Closed

timdeschryver mentioned this pull request May 7, 2019

strictActionSerializability does not work with RouterModule (and custom serializer) #1834

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Store): runtime checks #1613

feat(Store): runtime checks #1613

timdeschryver commented Mar 9, 2019 •

edited

Loading

timdeschryver Mar 9, 2019 •

edited

Loading

ngrxbot commented Mar 9, 2019

ngrxbot commented Mar 9, 2019

ngrxbot commented Mar 9, 2019

ngrxbot commented Mar 10, 2019

timdeschryver commented Mar 11, 2019

brandonroberts commented Mar 12, 2019

timdeschryver commented Mar 12, 2019 •

edited

Loading

timdeschryver commented Mar 12, 2019

ngrxbot commented Mar 12, 2019

timdeschryver commented Mar 12, 2019

ngrxbot commented Mar 13, 2019

brandonroberts commented Mar 14, 2019

brandonroberts Mar 14, 2019

brandonroberts Mar 14, 2019

ngrxbot commented Mar 16, 2019

ngrxbot commented Mar 16, 2019

ngrxbot commented Mar 21, 2019

brandonroberts Mar 21, 2019

brandonroberts Mar 21, 2019

brandonroberts Mar 21, 2019

timdeschryver Mar 21, 2019 •

edited

Loading

brandonroberts Mar 22, 2019

ngrxbot commented Mar 21, 2019

ngrxbot commented Mar 21, 2019

ngrxbot commented Mar 24, 2019

ngrxbot commented Mar 24, 2019

brandonroberts commented Mar 31, 2019

brandonroberts commented Mar 31, 2019 •

edited

Loading

timdeschryver commented Apr 1, 2019

ngrxbot commented Apr 1, 2019

feat(Store): runtime checks #1613

feat(Store): runtime checks #1613

Conversation

timdeschryver commented Mar 9, 2019 • edited Loading

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

timdeschryver Mar 9, 2019 • edited Loading

Choose a reason for hiding this comment

ngrxbot commented Mar 9, 2019

ngrxbot commented Mar 9, 2019

ngrxbot commented Mar 9, 2019

ngrxbot commented Mar 10, 2019

timdeschryver commented Mar 11, 2019

brandonroberts commented Mar 12, 2019

timdeschryver commented Mar 12, 2019 • edited Loading

timdeschryver commented Mar 12, 2019

ngrxbot commented Mar 12, 2019

timdeschryver commented Mar 12, 2019

ngrxbot commented Mar 13, 2019

brandonroberts commented Mar 14, 2019

brandonroberts Mar 14, 2019

Choose a reason for hiding this comment

brandonroberts Mar 14, 2019

Choose a reason for hiding this comment

ngrxbot commented Mar 16, 2019

ngrxbot commented Mar 16, 2019

ngrxbot commented Mar 21, 2019

brandonroberts Mar 21, 2019

Choose a reason for hiding this comment

brandonroberts Mar 21, 2019

Choose a reason for hiding this comment

brandonroberts Mar 21, 2019

Choose a reason for hiding this comment

timdeschryver Mar 21, 2019 • edited Loading

Choose a reason for hiding this comment

brandonroberts Mar 22, 2019

Choose a reason for hiding this comment

ngrxbot commented Mar 21, 2019

ngrxbot commented Mar 21, 2019

ngrxbot commented Mar 24, 2019

ngrxbot commented Mar 24, 2019

brandonroberts commented Mar 31, 2019

brandonroberts commented Mar 31, 2019 • edited Loading

timdeschryver commented Apr 1, 2019

ngrxbot commented Apr 1, 2019

timdeschryver commented Mar 9, 2019 •

edited

Loading

timdeschryver Mar 9, 2019 •

edited

Loading

timdeschryver commented Mar 12, 2019 •

edited

Loading

timdeschryver Mar 21, 2019 •

edited

Loading

brandonroberts commented Mar 31, 2019 •

edited

Loading