From 475bb2e98f16b283de77f62ec4d30487e21b34e4 Mon Sep 17 00:00:00 2001 From: Javier Gonzalez Date: Tue, 8 Oct 2019 19:13:31 +0200 Subject: [PATCH] fix recursivity, some edge cases --- src/action-async.ts | 30 +++++++++------ test/action-async.ts | 88 +++++++++++++++++++++++++++++++++++++++----- 2 files changed, 97 insertions(+), 21 deletions(-) diff --git a/src/action-async.ts b/src/action-async.ts index 082bafd..58a02cc 100644 --- a/src/action-async.ts +++ b/src/action-async.ts @@ -4,7 +4,8 @@ import { decorateMethodOrField } from "./decorator-utils" import { fail } from "./utils" let runId = 0 -const runningIds = new Set() +const unfinishedIds = new Set() +const currentlyActiveIds = new Set() interface IActionAsyncContext { runId: number @@ -38,12 +39,13 @@ export async function task(promise: Promise): Promise { const nextStep = step + 1 actionAsyncContextStack.pop() _endAction(actionRunInfo) + currentlyActiveIds.delete(runId) try { return await promise } finally { // only restart if it not a dangling promise (the action is not yet finished) - if (runningIds.has(runId)) { + if (unfinishedIds.has(runId)) { const actionRunInfo = _startAction( getActionAsyncName(actionName, runId, nextStep), this, @@ -58,6 +60,7 @@ export async function task(promise: Promise): Promise { args, scope }) + currentlyActiveIds.add(runId) } } } @@ -169,7 +172,7 @@ function actionAsyncFn(actionName: string, fn: Function): Function { return async function(this: any, ...args: any) { const nextRunId = runId++ - runningIds.add(nextRunId) + unfinishedIds.add(nextRunId) const actionRunInfo = _startAction(getActionAsyncName(actionName, nextRunId, 0), this, args) @@ -181,6 +184,7 @@ function actionAsyncFn(actionName: string, fn: Function): Function { args, scope: this }) + currentlyActiveIds.add(nextRunId) let errThrown: any try { @@ -190,17 +194,19 @@ function actionAsyncFn(actionName: string, fn: Function): Function { errThrown = err throw err } finally { - runningIds.delete(nextRunId) + unfinishedIds.delete(nextRunId) - const ctx = actionAsyncContextStack.pop() - if (!ctx || ctx.runId !== nextRunId) { - fail( - "'actionAsync' context not present or invalid. did you await inside an 'actionAsync' without using 'task(promise)'?" - ) + if (currentlyActiveIds.has(nextRunId)) { + const ctx = actionAsyncContextStack.pop() + if (!ctx || ctx.runId !== nextRunId) { + fail( + "'actionAsync' context not present or invalid. did you await inside an 'actionAsync' without using 'task(promise)'?" + ) + } + ctx.actionRunInfo.error = errThrown + _endAction(ctx.actionRunInfo) + currentlyActiveIds.delete(nextRunId) } - - ctx.actionRunInfo.error = errThrown - _endAction(ctx.actionRunInfo) } } } diff --git a/test/action-async.ts b/test/action-async.ts index 4dde803..4637397 100644 --- a/test/action-async.ts +++ b/test/action-async.ts @@ -17,6 +17,15 @@ function delayThrow(time: number, value: T) { }) } +function delayFn(time: number, fn: () => void) { + return new Promise(resolve => { + setTimeout(() => { + fn() + resolve() + }, time) + }) +} + function expectNoActionsRunning() { const obs = mobx.observable.box(1) const d = mobx.reaction(() => obs.get(), () => {}) @@ -326,7 +335,7 @@ test("dangling promises created indirectly inside the action should be ok", asyn expectNoActionsRunning() }) -test("dangling promises created directly inside the action using task should throw", async () => { +test("dangling promises created directly inside the action using task should be ok", async () => { mobx.configure({ enforceActions: "observed" }) let danglingP @@ -334,14 +343,7 @@ test("dangling promises created directly inside the action using task should thr danglingP = task(delay(100, 1)) // dangling promise }) - try { - await f1() - fail("should fail") - } catch (e) { - expect(e.message).toBe( - "[mobx-utils] 'actionAsync' context not present or invalid. did you await inside an 'actionAsync' without using 'task(promise)'?" - ) - } + await f1() expect(danglingP).toBeTruthy() await danglingP @@ -362,3 +364,71 @@ test("dangling promises created directly inside the action without using task be await danglingP expectNoActionsRunning() }) + +test("it should support recursive async (with task)", async () => { + mobx.configure({ enforceActions: "observed" }) + const values = [] + const x = mobx.observable({ a: 10 }) + mobx.reaction(() => x.a, v => values.push(v), { fireImmediately: true }) + + const f1 = actionAsync(async () => { + if (x.a <= 0) return + x.a -= await task(delay(10, 1)) + await task(f1()) + }) + + await f1() + expect(values).toEqual([10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0]) + expectNoActionsRunning() +}) + +test("it should support recursive async (without task)", async () => { + mobx.configure({ enforceActions: "observed" }) + const values = [] + const x = mobx.observable({ a: 10 }) + mobx.reaction(() => x.a, v => values.push(v), { fireImmediately: true }) + + const f1 = actionAsync(async () => { + if (x.a <= 0) return + x.a -= await task(delay(10, 1)) + await f1() + }) + + await f1() + expect(values).toEqual([10, 0]) + expectNoActionsRunning() +}) + +test("it should support parallel async", async () => { + mobx.configure({ enforceActions: "observed" }) + const values = [] + const x = mobx.observable({ a: 1 }) + mobx.reaction(() => x.a, v => values.push(v), { fireImmediately: true }) + + const f1 = actionAsync(async () => { + x.a = 2 + x.a = await task(delay(20, 5)) + x.a = await task(delay(40, 7)) + }) + + const f2 = actionAsync(async () => { + x.a = 3 + x.a = await task(delay(10, 4)) + x.a = await task(delay(30, 6)) + }) + + await Promise.all([ + f1(), + f2(), + async () => { + expectNoActionsRunning() + }, + delayFn(5, expectNoActionsRunning), + delayFn(15, expectNoActionsRunning), + delayFn(25, expectNoActionsRunning), + delayFn(35, expectNoActionsRunning), + delayFn(45, expectNoActionsRunning) + ]) + expect(values).toEqual([1, 2, 3, 4, 5, 6, 7]) + expectNoActionsRunning() +})