revert(store): store should fail synchronously (#1871)

Closes #1865 

* revert(store): should fail synchronously

This commit partially reverts commit 60633b7

* test(store): should fail synchronously

Author: timdeschryver

modules/store/spec/runtime_checks.spec.ts

@@ -1,5 +1,5 @@
 import * as ngCore from '@angular/core';
-import { TestBed } from '@angular/core/testing';
+import { TestBed, fakeAsync, flush } from '@angular/core/testing';
 import { Store, StoreModule, META_REDUCERS } from '..';
 import { createActiveRuntimeChecks } from '../src/runtime_checks';
 import { RuntimeChecks } from '../src/models';
@@ -113,32 +113,29 @@ describe('Runtime checks:', () => {
   describe('State Serialization:', () => {
     const invalidAction = () => ({ type: ErrorTypes.UnserializableState });
 
-    it('should throw when enabled', (done: DoneFn) => {
-      const store = setupStore({ strictStateSerializability: true });
-
-      store.subscribe({
-        error: err => {
-          expect(err).toMatch(/Detected unserializable state/);
-          done();
-        },
-      });
-
-      store.dispatch(invalidAction());
-    });
-
-    it('should not throw when disabled', (done: DoneFn) => {
-      const store = setupStore({ strictStateSerializability: false });
-
-      store.subscribe({
-        next: ({ state }) => {
-          if (state.invalidSerializationState) {
-            done();
-          }
-        },
-      });
-
-      store.dispatch(invalidAction());
-    });
+    it(
+      'should throw when enabled',
+      fakeAsync(() => {
+        const store = setupStore({ strictStateSerializability: true });
+
+        expect(() => {
+          store.dispatch(invalidAction());
+          flush();
+        }).toThrowError(/Detected unserializable state/);
+      })
+    );
+
+    it(
+      'should not throw when disabled',
+      fakeAsync(() => {
+        const store = setupStore({ strictStateSerializability: false });
+
+        expect(() => {
+          store.dispatch(invalidAction());
+          flush();
+        }).not.toThrow();
+      })
+    );
   });
 
   describe('Action Serialization:', () => {
@@ -147,63 +144,59 @@ describe('Runtime checks:', () => {
       invalid: new Date(),
     });
 
-    it('should throw when enabled', (done: DoneFn) => {
-      const store = setupStore({ strictActionSerializability: true });
-
-      store.subscribe({
-        error: err => {
-          expect(err).toMatch(/Detected unserializable action/);
-          done();
-        },
-      });
-      store.dispatch(invalidAction());
-    });
-
-    it('should not throw when disabled', (done: DoneFn) => {
-      const store = setupStore({ strictActionSerializability: false });
-
-      store.subscribe({
-        next: ({ state }) => {
-          if (state.invalidSerializationAction) {
-            done();
-          }
-        },
-      });
-
-      store.dispatch(invalidAction());
-    });
+    it(
+      'should throw when enabled',
+      fakeAsync(() => {
+        const store = setupStore({ strictActionSerializability: true });
+
+        expect(() => {
+          store.dispatch(invalidAction());
+          flush();
+        }).toThrowError(/Detected unserializable action/);
+      })
+    );
+
+    it(
+      'should not throw when disabled',
+      fakeAsync(() => {
+        const store = setupStore({ strictActionSerializability: false });
+
+        expect(() => {
+          store.dispatch(invalidAction());
+          flush();
+        }).not.toThrow();
+      })
+    );
   });
 
   describe('State Mutations', () => {
     const invalidAction = () => ({
       type: ErrorTypes.MutateState,
     });
 
-    it('should throw when enabled', (done: DoneFn) => {
-      const store = setupStore({ strictImmutability: true });
-
-      store.subscribe({
-        error: _ => {
-          done();
-        },
-      });
-
-      store.dispatch(invalidAction());
-    });
-
-    it('should not throw when disabled', (done: DoneFn) => {
-      const store = setupStore({ strictImmutability: false });
-
-      store.subscribe({
-        next: ({ state }) => {
-          if (state.invalidMutationState) {
-            done();
-          }
-        },
-      });
-
-      store.dispatch(invalidAction());
-    });
+    it(
+      'should throw when enabled',
+      fakeAsync(() => {
+        const store = setupStore({ strictImmutability: true });
+
+        expect(() => {
+          store.dispatch(invalidAction());
+          flush();
+        }).toThrowError(/Cannot add property/);
+      })
+    );
+
+    it(
+      'should not throw when disabled',
+      fakeAsync(() => {
+        const store = setupStore({ strictImmutability: false });
+
+        expect(() => {
+          store.dispatch(invalidAction());
+          flush();
+        }).not.toThrow();
+      })
+    );
   });
 
   describe('Action Mutations', () => {
@@ -212,31 +205,29 @@ describe('Runtime checks:', () => {
       foo: 'foo',
     });
 
-    it('should throw when enabled', (done: DoneFn) => {
-      const store = setupStore({ strictImmutability: true });
-
-      store.subscribe({
-        error: _ => {
-          done();
-        },
-      });
-
-      store.dispatch(invalidAction());
-    });
-
-    it('should not throw when disabled', (done: DoneFn) => {
-      const store = setupStore({ strictImmutability: false });
-
-      store.subscribe({
-        next: ({ state }) => {
-          if (state.invalidMutationAction) {
-            done();
-          }
-        },
-      });
-
-      store.dispatch(invalidAction());
-    });
+    it(
+      'should throw when enabled',
+      fakeAsync(() => {
+        const store = setupStore({ strictImmutability: true });
+
+        expect(() => {
+          store.dispatch(invalidAction());
+          flush();
+        }).toThrowError(/Cannot assign to read only property/);
+      })
+    );
+
+    it(
+      'should not throw when disabled',
+      fakeAsync(() => {
+        const store = setupStore({ strictImmutability: false });
+
+        expect(() => {
+          store.dispatch(invalidAction());
+          flush();
+        }).not.toThrow();
+      })
+    );
   });
 });
 

modules/store/spec/state.spec.ts

@@ -1,11 +1,11 @@
-import { TestBed } from '@angular/core/testing';
-import { INIT, Store, StoreModule } from '@ngrx/store';
+import { TestBed, fakeAsync, flush } from '@angular/core/testing';
+import { INIT, Store, StoreModule, Action } from '@ngrx/store';
 
 describe('ngRx State', () => {
-  const initialState = 123;
-  const reducer = jasmine.createSpy('reducer').and.returnValue(initialState);
+  it('should call the reducer to scan over the dispatcher', () => {
+    const initialState = 123;
+    const reducer = jasmine.createSpy('reducer').and.returnValue(initialState);
 
-  beforeEach(() => {
     TestBed.configureTestingModule({
       imports: [
         StoreModule.forRoot(
@@ -14,13 +14,34 @@ describe('ngRx State', () => {
         ),
       ],
     });
-  });
 
-  it('should call the reducer to scan over the dispatcher', function() {
     TestBed.get(Store);
 
     expect(reducer).toHaveBeenCalledWith(initialState, {
       type: INIT,
     });
   });
+
+  it(
+    'should fail synchronously',
+    fakeAsync(() => {
+      function reducer(state: any, action: Action) {
+        if (action.type === 'THROW_ERROR') {
+          throw new Error('(╯°□°）╯︵ ┻━┻');
+        }
+
+        return state;
+      }
+
+      TestBed.configureTestingModule({
+        imports: [StoreModule.forRoot({ reducer })],
+      });
+
+      const store = TestBed.get(Store) as Store<any>;
+      expect(() => {
+        store.dispatch({ type: 'THROW_ERROR' });
+        flush();
+      }).toThrow();
+    })
+  );
 });

modules/store/src/state.ts

@@ -47,12 +47,9 @@ export class State<T> extends BehaviorSubject<any> implements OnDestroy {
       )
     );
 
-    this.stateSubscription = stateAndAction$.subscribe({
-      next: ({ state, action }) => {
-        this.next(state);
-        scannedActions.next(action);
-      },
-      error: err => this.error(err),
+    this.stateSubscription = stateAndAction$.subscribe(({ state, action }) => {
+      this.next(state);
+      scannedActions.next(action);
     });
   }