From 59a9e6c77f627fcd75f8d00dfc9e6b7f05314989 Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Fri, 17 May 2019 20:32:58 +0200 Subject: [PATCH] revert(store): store should fail synchronously (#1871) Closes #1865 * revert(store): should fail synchronously This commit partially reverts commit 60633b770e27f6d984abc91a802df5ea9902d80a * test(store): should fail synchronously --- modules/store/spec/runtime_checks.spec.ts | 195 +++++++++++----------- modules/store/spec/state.spec.ts | 35 +++- modules/store/src/state.ts | 9 +- 3 files changed, 124 insertions(+), 115 deletions(-) diff --git a/modules/store/spec/runtime_checks.spec.ts b/modules/store/spec/runtime_checks.spec.ts index 23a4fddec0..d4d1b4688c 100644 --- a/modules/store/spec/runtime_checks.spec.ts +++ b/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,31 +144,29 @@ 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', () => { @@ -179,31 +174,29 @@ describe('Runtime checks:', () => { 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(); + }) + ); }); }); diff --git a/modules/store/spec/state.spec.ts b/modules/store/spec/state.spec.ts index 36f38fb742..9693fe17fa 100644 --- a/modules/store/spec/state.spec.ts +++ b/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; + expect(() => { + store.dispatch({ type: 'THROW_ERROR' }); + flush(); + }).toThrow(); + }) + ); }); diff --git a/modules/store/src/state.ts b/modules/store/src/state.ts index 0eeba2dd25..c0a7fc9b50 100644 --- a/modules/store/src/state.ts +++ b/modules/store/src/state.ts @@ -47,12 +47,9 @@ export class State extends BehaviorSubject 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); }); }