feat(store): add strictActionWithinNgZone runtime check (#2364)

Closes #2339
ngrx · Feb 11, 2020 · 4cae255 · 4cae255
1 parent 234ce84
commit 4cae255
Show file tree

Hide file tree

Showing 11 changed files with 196 additions and 14 deletions.
diff --git a/modules/store/spec/meta-reducers/inNgZoneAssert_reducer.spec.ts b/modules/store/spec/meta-reducers/inNgZoneAssert_reducer.spec.ts
@@ -0,0 +1,36 @@
+import * as ngCore from '@angular/core';
+import { inNgZoneAssertMetaReducer } from '../../src/meta-reducers';
+
+describe('inNgZoneAssertMetaReducer:', () => {
+  it('should not throw if in NgZone', () => {
+    ngCore.NgZone.isInAngularZone = jasmine
+      .createSpy('isInAngularZone')
+      .and.returnValue(true);
+    expect(() => invokeActionReducer((state: any) => state)).not.toThrow();
+    expect(ngCore.NgZone.isInAngularZone).toHaveBeenCalled();
+  });
+
+  it('should throw when not in NgZone', () => {
+    ngCore.NgZone.isInAngularZone = jasmine
+      .createSpy('isInAngularZone')
+      .and.returnValue(false);
+    expect(() => invokeActionReducer((state: any) => state)).toThrowError(
+      `Action 'invoke' running outside NgZone. https://ngrx.io/guide/store/configuration/runtime-checks#strictactionwithinngzone`
+    );
+    expect(ngCore.NgZone.isInAngularZone).toHaveBeenCalled();
+  });
+
+  it('should not call isInAngularZone when check is off', () => {
+    ngCore.NgZone.isInAngularZone = jasmine.createSpy('isInAngularZone');
+    expect(() =>
+      invokeActionReducer((state: any) => state, false)
+    ).not.toThrow();
+    expect(ngCore.NgZone.isInAngularZone).not.toHaveBeenCalled();
+  });
+
+  function invokeActionReducer(reduce: Function, checkIsOn = true) {
+    inNgZoneAssertMetaReducer((state, action) => reduce(state, action), {
+      action: () => checkIsOn,
+    })({}, { type: 'invoke' });
+  }
+});
diff --git a/modules/store/spec/meta-reducers/serialization_reducer.spec.ts b/modules/store/spec/meta-reducers/serialization_reducer.spec.ts
@@ -62,7 +62,7 @@ describe('serializationCheckMetaReducer:', () => {
           payload: { foo: { bar: unSerializables['date'] } },
         })
       ).toThrowError(
-        /Detected unserializable action at "payload.foo.bar.value"/
+        `Detected unserializable action at "payload.foo.bar.value". https://ngrx.io/guide/store/configuration/runtime-checks#strictactionserializability`
       );
     });
   });
@@ -80,13 +80,13 @@ describe('serializationCheckMetaReducer:', () => {
       ).toThrowError(/Detected unserializable state at "foo.bar.value"/);
     });
 
-    it('should not throw if state is null', () => {
+    it('should throw if state is null', () => {
       expect(() => invokeStateReducer(null)).toThrowError(
-        /Detected unserializable state at "root"/
+        `Detected unserializable state at "root". https://ngrx.io/guide/store/configuration/runtime-checks#strictstateserializability`
       );
     });
 
-    it('should not throw if state is undefined', () => {
+    it('should throw if state is undefined', () => {
       expect(() => invokeStateReducer(undefined)).toThrowError(
         /Detected unserializable state at "root"/
       );

diff --git a/modules/store/spec/runtime_checks.spec.ts b/modules/store/spec/runtime_checks.spec.ts
@@ -13,6 +13,7 @@ describe('Runtime checks:', () => {
         strictActionSerializability: false,
         strictActionImmutability: true,
         strictStateImmutability: true,
+        strictActionWithinNgZone: false,
       });
     });
 
@@ -23,12 +24,14 @@ describe('Runtime checks:', () => {
           strictActionSerializability: true,
           strictActionImmutability: false,
           strictStateImmutability: false,
+          strictActionWithinNgZone: true,
         })
       ).toEqual({
         strictStateSerializability: true,
         strictActionSerializability: true,
         strictActionImmutability: false,
         strictStateImmutability: false,
+        strictActionWithinNgZone: true,
       });
     });
 
@@ -40,6 +43,7 @@ describe('Runtime checks:', () => {
         strictActionSerializability: false,
         strictActionImmutability: false,
         strictStateImmutability: false,
+        strictActionWithinNgZone: false,
       });
     });
 
@@ -50,12 +54,14 @@ describe('Runtime checks:', () => {
         createActiveRuntimeChecks({
           strictStateSerializability: true,
           strictActionSerializability: true,
+          strictActionWithinNgZone: true,
         })
       ).toEqual({
         strictStateSerializability: false,
         strictActionSerializability: false,
         strictActionImmutability: false,
         strictStateImmutability: false,
+        strictActionWithinNgZone: false,
       });
     });
   });
@@ -88,6 +94,10 @@ describe('Runtime checks:', () => {
         metaReducers,
         'serializationCheckMetaReducer'
       ).and.callThrough();
+      const inNgZoneAssertMetaReducerSpy = spyOn(
+        metaReducers,
+        'inNgZoneAssertMetaReducer'
+      ).and.callThrough();
 
       TestBed.configureTestingModule({
         imports: [StoreModule.forRoot({})],
@@ -96,13 +106,15 @@ describe('Runtime checks:', () => {
             provide: USER_RUNTIME_CHECKS,
             useValue: {
               strictStateSerializability: false,
+              strictActionWithinNgZone: false,
             },
           },
         ],
       });
 
       const _store = TestBed.get<Store<any>>(Store);
       expect(serializationCheckMetaReducerSpy).not.toHaveBeenCalled();
+      expect(inNgZoneAssertMetaReducerSpy).not.toHaveBeenCalled();
     });
 
     it('should create immutability meta reducer without config', () => {
@@ -320,6 +332,56 @@ describe('Runtime checks:', () => {
       })
     );
   });
+
+  describe('Action in NgZone', () => {
+    const invalidAction = () => ({ type: ErrorTypes.OutOfNgZoneAction });
+
+    it(
+      'should throw when running outside ngZone',
+      fakeAsync(() => {
+        ngCore.NgZone.isInAngularZone = jasmine
+          .createSpy('isInAngularZone')
+          .and.returnValue(false);
+        const store = setupStore({ strictActionWithinNgZone: true });
+        expect(() => {
+          store.dispatch(invalidAction());
+          flush();
+        }).toThrowError(
+          "Action 'Action triggered outside of NgZone' running outside NgZone. https://ngrx.io/guide/store/configuration/runtime-checks#strictactionwithinngzone"
+        );
+      })
+    );
+
+    it(
+      'should not throw when running in ngZone',
+      fakeAsync(() => {
+        ngCore.NgZone.isInAngularZone = jasmine
+          .createSpy('isInAngularZone')
+          .and.returnValue(true);
+        const store = setupStore({ strictActionWithinNgZone: true });
+        expect(() => {
+          store.dispatch(invalidAction());
+          flush();
+        }).not.toThrowError();
+
+        expect(ngCore.NgZone.isInAngularZone).toHaveBeenCalled();
+      })
+    );
+
+    it(
+      'should not be called when disabled',
+      fakeAsync(() => {
+        const store = setupStore({ strictActionWithinNgZone: false });
+        ngCore.NgZone.isInAngularZone = jasmine.createSpy('isInAngularZone');
+        expect(() => {
+          store.dispatch(invalidAction());
+          flush();
+        }).not.toThrow();
+
+        expect(ngCore.NgZone.isInAngularZone).not.toHaveBeenCalled();
+      })
+    );
+  });
 });
 
 function setupStore(runtimeChecks?: Partial<RuntimeChecks>): Store<any> {
@@ -342,6 +404,7 @@ enum ErrorTypes {
   UnserializableAction = 'Action type producing unserializable action',
   MutateAction = 'Action type producing action mutation',
   MutateState = 'Action type producing state mutation',
+  OutOfNgZoneAction = 'Action triggered outside of NgZone',
 }
 
 function reducerWithBugs(state: any = {}, action: any) {

diff --git a/modules/store/src/meta-reducers/inNgZoneAssert_reducer.ts b/modules/store/src/meta-reducers/inNgZoneAssert_reducer.ts
@@ -0,0 +1,19 @@
+import * as ngCore from '@angular/core';
+import { Action, ActionReducer } from '../models';
+import { RUNTIME_CHECK_URL } from './utils';
+
+export function inNgZoneAssertMetaReducer(
+  reducer: ActionReducer<any, Action>,
+  checks: { action: (action: Action) => boolean }
+) {
+  return function(state: any, action: Action) {
+    if (checks.action(action) && !ngCore.NgZone.isInAngularZone()) {
+      throw new Error(
+        `Action '${
+          action.type
+        }' running outside NgZone. ${RUNTIME_CHECK_URL}#strictactionwithinngzone`
+      );
+    }
+    return reducer(state, action);
+  };
+}
diff --git a/modules/store/src/meta-reducers/index.ts b/modules/store/src/meta-reducers/index.ts
@@ -1,2 +1,3 @@
 export { immutabilityCheckMetaReducer } from './immutability_reducer';
 export { serializationCheckMetaReducer } from './serialization_reducer';
+export { inNgZoneAssertMetaReducer } from './inNgZoneAssert_reducer';
diff --git a/modules/store/src/meta-reducers/serialization_reducer.ts b/modules/store/src/meta-reducers/serialization_reducer.ts
@@ -7,6 +7,7 @@ import {
   isBoolean,
   isString,
   isArray,
+  RUNTIME_CHECK_URL,
 } from './utils';
 
 export function serializationCheckMetaReducer(
@@ -82,7 +83,7 @@ function throwIfUnserializable(
 
   const unserializablePath = unserializable.path.join('.');
   const error: any = new Error(
-    `Detected unserializable ${context} at "${unserializablePath}"`
+    `Detected unserializable ${context} at "${unserializablePath}". ${RUNTIME_CHECK_URL}#strict${context}serializability`
   );
   error.value = unserializable.value;
   error.unserializablePath = unserializablePath;

diff --git a/modules/store/src/meta-reducers/utils.ts b/modules/store/src/meta-reducers/utils.ts
@@ -1,3 +1,6 @@
+export const RUNTIME_CHECK_URL =
+  'https://ngrx.io/guide/store/configuration/runtime-checks';
+
 export function isUndefined(target: any): target is undefined {
   return target === undefined;
 }

diff --git a/modules/store/src/models.ts b/modules/store/src/models.ts
@@ -115,4 +115,9 @@ export interface RuntimeChecks {
    * Verifies that actions aren't mutated
    */
   strictActionImmutability: boolean;
+
+  /**
+   * Verifies that actions are dispatched within NgZone
+   */
+  strictActionWithinNgZone: boolean;
 }
diff --git a/modules/store/src/runtime_checks.ts b/modules/store/src/runtime_checks.ts
@@ -2,6 +2,7 @@ import { isDevMode, Provider } from '@angular/core';
 import {
   serializationCheckMetaReducer,
   immutabilityCheckMetaReducer,
+  inNgZoneAssertMetaReducer,
 } from './meta-reducers';
 import { RuntimeChecks, MetaReducer, Action } from './models';
 import {
@@ -20,6 +21,7 @@ export function createActiveRuntimeChecks(
       strictActionSerializability: false,
       strictStateImmutability: true,
       strictActionImmutability: true,
+      strictActionWithinNgZone: false,
       ...runtimeChecks,
     };
   }
@@ -29,6 +31,7 @@ export function createActiveRuntimeChecks(
     strictActionSerializability: false,
     strictStateImmutability: false,
     strictActionImmutability: false,
+    strictActionWithinNgZone: false,
   };
 }
 
@@ -64,6 +67,18 @@ function ignoreNgrxAction(action: Action) {
   return action.type.startsWith('@ngrx');
 }
 
+export function createInNgZoneCheckMetaReducer({
+  strictActionWithinNgZone,
+}: RuntimeChecks): MetaReducer {
+  return reducer =>
+    strictActionWithinNgZone
+      ? inNgZoneAssertMetaReducer(reducer, {
+          action: action =>
+            strictActionWithinNgZone && !ignoreNgrxAction(action),
+        })
+      : reducer;
+}
+
 export function provideRuntimeChecks(
   runtimeChecks?: Partial<RuntimeChecks>
 ): Provider[] {
@@ -94,6 +109,12 @@ export function provideRuntimeChecks(
       deps: [_ACTIVE_RUNTIME_CHECKS],
       useFactory: createSerializationCheckMetaReducer,
     },
+    {
+      provide: META_REDUCERS,
+      multi: true,
+      deps: [_ACTIVE_RUNTIME_CHECKS],
+      useFactory: createInNgZoneCheckMetaReducer,
+    },
   ];
 }
 

diff --git a/projects/example-app/src/app/app.module.ts b/projects/example-app/src/app/app.module.ts
@@ -41,6 +41,7 @@ import { AppComponent } from '@example-app/core/containers';
         strictActionImmutability: true,
         strictStateSerializability: true,
         strictActionSerializability: true,
+        strictActionWithinNgZone: true,
       },
     }),
 

diff --git a/projects/ngrx.io/content/guide/store/configuration/runtime-checks.md b/projects/ngrx.io/content/guide/store/configuration/runtime-checks.md
@@ -2,18 +2,21 @@
 
 Runtime checks are here to guide developers to follow the NgRx and Redux core concepts and best practices. They are here to shorten the feedback loop of easy-to-make mistakes when you're starting to use NgRx, or even a well-seasoned developer might make. During development, when a rule is violated, an error is thrown notifying you what and where something went wrong.
 
-`@ngrx/store` ships with four (4) built-in runtime checks:
+`@ngrx/store` ships with five (5) built-in runtime checks:
 
-- [`strictStateImmutability`](#strictstateimmutability): verifies that the state isn't mutated
-- [`strictActionImmutability`](#strictactionimmutability): verifies that actions aren't mutated
-- [`strictStateSerializability`](#strictstateserializability): verifies if the state is serializable
-- [`strictActionSerializability`](#strictactionserializability): verifies if the actions are serializable
+- Default On:
+  - [`strictStateImmutability`](#strictstateimmutability): verifies that the state isn't mutated.
+  - [`strictActionImmutability`](#strictactionimmutability): verifies that actions aren't mutated
+- Default Off:
+  - [`strictStateSerializability`](#strictstateserializability): verifies if the state is serializable
+  - [`strictActionSerializability`](#strictactionserializability): verifies if the actions are serializable
+  - [`strictActionWithinNgZone`](#strictactionwithinngzone): verifies if actions are dispatched within NgZone
 
-These checks are all opt-in and will automatically be disabled in production builds.
+All checks will automatically be disabled in production builds.
 
-## Enabling runtime checks
+## Configuring runtime checks
 
-It's possible to turn on the runtime checks one by one. To do so, you must enable them while providing the root store. Use the `runtimeChecks` property on the root store's config object. For each runtime check you can toggle the check with a `boolean`, `true` to enable the check, `false` to disable the check.
+It's possible to override the default configuration of runtime checks. To do so, use the `runtimeChecks` property on the root store's config object. For each runtime check you can toggle the check with a `boolean`, `true` to enable the check, `false` to disable the check.
 
 ```ts
 @NgModule({
@@ -24,6 +27,7 @@ It's possible to turn on the runtime checks one by one. To do so, you must enabl
         strictActionImmutability: true,
         strictStateSerializability: true,
         strictActionSerializability: true,
+        strictActionWithinNgZone: true
       },
     }),
   ],
@@ -170,4 +174,32 @@ function logTodo (todo: Todo) {
 
 Please note, you may not need to set `strictActionSerializability` to `true` unless you are storing/replaying actions using external resources, for example `localStorage`.
 
-</div>
+</div>
+
+### strictActionWithinNgZone
+
+The `strictActionWithinNgZone` check verifies that Actions are dispatched by asynchronous tasks running within `NgZone`. Actions dispatched by tasks, running outside of `NgZone`, will not trigger ChangeDetection upon completion and may result in a stale view.
+
+Example violation of the rule:
+
+```ts
+// Callback running outside of NgZone
+function callbackOutsideNgZone(){
+  this.store.dispatch(clearTodos());
+}
+```
+
+To fix ensure actions are running within `NgZone`. Identify the event trigger and then verify if the code can be updated to use a `NgZone` aware feature. If this is not possible use the `NgZone.run` method to explicitly run the asynchronous task within NgZone.
+
+```ts
+import { NgZone } from '@angular/core';
+
+constructor(private ngZone: NgZone){}
+
+// Callback running outside of NgZone brought back in NgZone.
+function callbackOutsideNgZone(){
+  this.ngZone.run(() => {
+    this.store.dispatch(clearTodos());
+  }
+}
+```