From e964670a63afa7a934231883553962f0607d7836 Mon Sep 17 00:00:00 2001 From: jarmit Date: Fri, 20 Mar 2020 14:44:38 -0700 Subject: [PATCH] Ensure mutators don't return a type through a build time error instead of a run time error (#140) * Ensure mutators don't return a type through a build time error instead of a run time error --- package.json | 4 ++-- src/dispatcher.ts | 2 +- src/globalContext.ts | 2 +- src/interfaces/MutatorFunction.ts | 6 +++++- src/interfaces/SimpleAction.ts | 5 +---- src/interfaces/Subscriber.ts | 4 +++- src/mutator.ts | 14 ++++++-------- src/simpleSubscribers.ts | 7 +++++-- test/endToEndTests.ts | 4 ++-- test/mutatorTests.ts | 12 ------------ test/simpleSubscribersTests.ts | 9 --------- 11 files changed, 26 insertions(+), 43 deletions(-) diff --git a/package.json b/package.json index abf4bdb..91375f3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "satcheljs", - "version": "4.2.0", + "version": "4.2.1", "description": "Store implementation for functional reactive flux.", "lint-staged": { "*.{ts,tsx}": [ @@ -17,7 +17,7 @@ "test:unit": "jest", "build": "run-s clean build:source", "start": "run-s clean watch", - "test": "run-s lint test:unit", + "test": "run-s lint build test:unit", "test:start": "jest --watch" }, "devDependencies": { diff --git a/src/dispatcher.ts b/src/dispatcher.ts index c471b35..92d71f7 100644 --- a/src/dispatcher.ts +++ b/src/dispatcher.ts @@ -4,7 +4,7 @@ import Subscriber from './interfaces/Subscriber'; import { getPrivateActionId } from './actionCreator'; import { getGlobalContext } from './globalContext'; -export function subscribe(actionId: string, callback: Subscriber) { +export function subscribe(actionId: string, callback: Subscriber) { let subscriptions = getGlobalContext().subscriptions; if (!subscriptions[actionId]) { subscriptions[actionId] = []; diff --git a/src/globalContext.ts b/src/globalContext.ts index a2b852b..6ae91ab 100644 --- a/src/globalContext.ts +++ b/src/globalContext.ts @@ -12,7 +12,7 @@ export interface GlobalContext { schemaVersion: number; rootStore: ObservableMap; nextActionId: number; - subscriptions: { [key: string]: Subscriber[] }; + subscriptions: { [key: string]: Subscriber[] }; dispatchWithMiddleware: DispatchFunction; currentMutator: string | null; diff --git a/src/interfaces/MutatorFunction.ts b/src/interfaces/MutatorFunction.ts index 7b18b4a..fc79f9a 100644 --- a/src/interfaces/MutatorFunction.ts +++ b/src/interfaces/MutatorFunction.ts @@ -1,4 +1,8 @@ import ActionMessage from './ActionMessage'; -type MutatorFunction = (actionMessage: T) => void; +// if the return type is void, then it can be a function. Or else it should never happen. +// this is how we ensure that all functions passed in have a return type of void +type MutatorFunction = void extends TReturn + ? (actionMessage: TAction) => TReturn + : never; export default MutatorFunction; diff --git a/src/interfaces/SimpleAction.ts b/src/interfaces/SimpleAction.ts index 97b6471..4eae91c 100644 --- a/src/interfaces/SimpleAction.ts +++ b/src/interfaces/SimpleAction.ts @@ -1,5 +1,2 @@ -interface SimpleAction { - (...args: any[]): void; -} - +type SimpleAction = void extends TReturn ? (...args: any[]) => TReturn : never; export default SimpleAction; diff --git a/src/interfaces/Subscriber.ts b/src/interfaces/Subscriber.ts index 655cff0..66cfbc3 100644 --- a/src/interfaces/Subscriber.ts +++ b/src/interfaces/Subscriber.ts @@ -2,5 +2,7 @@ import ActionMessage from './ActionMessage'; import MutatorFunction from './MutatorFunction'; import OrchestratorFunction from './OrchestratorFunction'; -type Subscriber = MutatorFunction | OrchestratorFunction; +type Subscriber = + | MutatorFunction + | OrchestratorFunction; export default Subscriber; diff --git a/src/mutator.ts b/src/mutator.ts index 86e5ce7..b148fd4 100644 --- a/src/mutator.ts +++ b/src/mutator.ts @@ -7,22 +7,20 @@ import { getPrivateActionType, getPrivateActionId } from './actionCreator'; import { subscribe } from './dispatcher'; import { getGlobalContext } from './globalContext'; -export default function mutator( - actionCreator: ActionCreator, - target: MutatorFunction -): MutatorFunction { +export default function mutator( + actionCreator: ActionCreator, + target: MutatorFunction +): MutatorFunction { let actionId = getPrivateActionId(actionCreator); if (!actionId) { throw new Error('Mutators can only subscribe to action creators.'); } // Wrap the callback in a MobX action so it can modify the store - let wrappedTarget = action(getPrivateActionType(actionCreator), (actionMessage: T) => { + let wrappedTarget = action(getPrivateActionType(actionCreator), (actionMessage: TAction) => { try { getGlobalContext().currentMutator = actionCreator.name; - if (target(actionMessage) as any) { - throw new Error('Mutators cannot return a value and cannot be async.'); - } + target(actionMessage); } finally { getGlobalContext().currentMutator = null; } diff --git a/src/simpleSubscribers.ts b/src/simpleSubscribers.ts index 4040b1f..df7c0d9 100644 --- a/src/simpleSubscribers.ts +++ b/src/simpleSubscribers.ts @@ -3,7 +3,10 @@ import { action } from './actionCreator'; import mutator from './mutator'; export function createSimpleSubscriber(decorator: Function) { - return function simpleSubscriber(actionType: string, target: T): T { + return function simpleSubscriber( + actionType: string, + target: SimpleAction + ): SimpleAction { // Create the action creator let simpleActionCreator = action(actionType, function simpleActionCreator() { return { @@ -17,7 +20,7 @@ export function createSimpleSubscriber(decorator: Function) { }); // Return a function that dispatches that action - return (simpleActionCreator as any) as T; + return (simpleActionCreator as any) as SimpleAction; }; } diff --git a/test/endToEndTests.ts b/test/endToEndTests.ts index 1ce8f85..e033480 100644 --- a/test/endToEndTests.ts +++ b/test/endToEndTests.ts @@ -27,7 +27,7 @@ describe('satcheljs', () => { }); // Create a mutator that subscribes to it - mutator(testAction, function(actionMessage) { + mutator(testAction, function(actionMessage: any) { actualValue = actionMessage.value; }); @@ -43,7 +43,7 @@ describe('satcheljs', () => { let arg1Value; let arg2Value; - let testMutatorAction = mutatorAction('testMutatorAction', function testMutatorAction( + let testMutatorAction = mutatorAction('testMutatorAction', function testMutatorAction( arg1: string, arg2: number ) { diff --git a/test/mutatorTests.ts b/test/mutatorTests.ts index 4e57c82..d67ea6e 100644 --- a/test/mutatorTests.ts +++ b/test/mutatorTests.ts @@ -62,18 +62,6 @@ describe('mutator', () => { expect(returnValue).toBe(callback); }); - it('throws if the target function is async', () => { - // Arrange - let actionCreator: any = { __SATCHELJS_ACTION_ID: 'testAction' }; - let callback = async () => {}; - - mutator(actionCreator, callback); - let subscribedCallback = (dispatcher.subscribe as jasmine.Spy).calls.argsFor(0)[1]; - - // Act / Assert - expect(subscribedCallback).toThrow(); - }); - it('sets the currentMutator to actionMessage type for the duration of the mutator callback', () => { // Arrange let actionCreator: any = { __SATCHELJS_ACTION_ID: 'testAction', name: 'testName' }; diff --git a/test/simpleSubscribersTests.ts b/test/simpleSubscribersTests.ts index d8e2bc7..b6793a1 100644 --- a/test/simpleSubscribersTests.ts +++ b/test/simpleSubscribersTests.ts @@ -59,13 +59,4 @@ describe('simpleSubscribers', () => { // Assert expect(callback).toHaveBeenCalledWith(1, 2, 3); }); - - it('throws if the target function is async', () => { - // Arrange - let callback = async () => {}; - let actionCreator = mutatorAction('testMutator', callback); - - // Act / Assert - expect(actionCreator).toThrow(); - }); });