From 50591e0b8eceb7b9cf5cdd2d7d3e4e63c57004f4 Mon Sep 17 00:00:00 2001 From: sakuntala_motukuri Date: Tue, 16 Jul 2024 02:53:05 -0400 Subject: [PATCH] Rebased onto main branch for ability to remove a resolved async node --- plugins/async-node/core/src/index.test.ts | 98 +++++++++++++++-------- plugins/async-node/core/src/index.ts | 31 ++++--- 2 files changed, 86 insertions(+), 43 deletions(-) diff --git a/plugins/async-node/core/src/index.test.ts b/plugins/async-node/core/src/index.test.ts index 74c1e0f4d..81e0df109 100644 --- a/plugins/async-node/core/src/index.test.ts +++ b/plugins/async-node/core/src/index.test.ts @@ -83,20 +83,17 @@ const asyncNodeTest = async (resolvedValue: any) => { deferredResolve(resolvedValue); } - await waitFor(() => { - expect(updateNumber).toBe(2); - }); - view = (player.getState() as InProgressState).controllers.view.currentView ?.lastUpdate; expect(view?.actions[0].asset.type).toBe("action"); expect(view?.actions.length).toBe(1); - viewInstance?.update(); + viewInstance.update(); + //Even after a manual update, the view should not change as we are deleting the resolved node if there is no view update await waitFor(() => { - expect(updateNumber).toBe(3); + expect(updateNumber).toBe(2); }); view = (player.getState() as InProgressState).controllers.view.currentView @@ -114,6 +111,66 @@ test("should return current node view when the resolved node is undefined", asyn await asyncNodeTest(undefined); }); +test("should handle the promise using .then", async () => { + const plugin = new AsyncNodePlugin({ + plugins: [new AsyncNodePluginPlugin()], + }); + + let deferredResolve: ((value: any) => void) | undefined; + + plugin.hooks.onAsyncNode.tap("test", async (node: Node.Node) => { + return new Promise((resolve) => { + deferredResolve = resolve; + }); + }); + let updateNumber = 0; + + const player = new Player({ plugins: [plugin] }); + + player.hooks.viewController.tap("async-node-test", (vc) => { + vc.hooks.view.tap("async-node-test", (view) => { + view.hooks.onUpdate.tap("async-node-test", (update) => { + updateNumber++; + }); + }); + }); + + player.start(basicFRFWithActions as any); + + let view = (player.getState() as InProgressState).controllers.view.currentView + ?.lastUpdate; + + expect(view).toBeDefined(); + expect(view?.actions[1]).toBeUndefined(); + + await waitFor(() => { + expect(updateNumber).toBe(1); + expect(deferredResolve).toBeDefined(); + }); + + if (deferredResolve) { + deferredResolve({ + asset: { + id: "next-label-action", + type: "action", + value: "dummy value", + }, + }); + } + + await waitFor(() => { + expect(updateNumber).toBe(2); + }); + + view = (player.getState() as InProgressState).controllers.view.currentView + ?.lastUpdate; + + expect(view?.actions[0].asset.type).toBe("action"); + expect(view?.actions[1].asset.type).toBe("action"); + expect(view?.actions[2]).toBeUndefined(); + expect(updateNumber).toBe(2); +}); + test("replaces async nodes with provided node", async () => { const plugin = new AsyncNodePlugin({ plugins: [new AsyncNodePluginPlugin()], @@ -346,7 +403,7 @@ test("replaces async nodes with chained multiNodes singular", async () => { let deferredResolve: ((value: any) => void) | undefined; - plugin.hooks.onAsyncNode.tap("test", async (node: Node.Async) => { + plugin.hooks.onAsyncNode.tap("test", async (node: Node.Node) => { return new Promise((resolve) => { deferredResolve = resolve; }); @@ -424,30 +481,3 @@ test("replaces async nodes with chained multiNodes singular", async () => { expect(view?.actions[1].asset.type).toBe("text"); expect(view?.actions[2].asset.type).toBe("text"); }); - -test("should call onAsyncNode hook when async node is encountered", async () => { - const plugin = new AsyncNodePlugin({ - plugins: [new AsyncNodePluginPlugin()], - }); - - let localNode: Node.Async; - plugin.hooks.onAsyncNode.tap("test", async (node: Node.Async) => { - if (node !== null) { - // assigns node value to a local variable - localNode = node; - } - - return new Promise((resolve) => { - resolve("Promise resolved"); - }); - }); - - const player = new Player({ plugins: [plugin] }); - - player.start(basicFRFWithActions as any); - - await waitFor(() => { - expect(localNode.id).toStrictEqual("nodeId"); - expect(localNode.type).toStrictEqual("async"); - }); -}); diff --git a/plugins/async-node/core/src/index.ts b/plugins/async-node/core/src/index.ts index a8f63c051..2dde59896 100644 --- a/plugins/async-node/core/src/index.ts +++ b/plugins/async-node/core/src/index.ts @@ -70,6 +70,8 @@ export class AsyncNodePluginPlugin implements AsyncNodeViewPlugin { private currentView: ViewInstance | undefined; + private pendingUpdates = new Set(); + private isAsync(node: Node.Node | null): node is Node.Async { return node?.type === NodeType.Async; } @@ -151,8 +153,12 @@ export class AsyncNodePluginPlugin implements AsyncNodeViewPlugin { let resolvedNode; if (this.isAsync(node)) { const mappedValue = this.resolvedMapping.get(node.id); + if (this.pendingUpdates.has(node.id)) { + return; // Skip processing if already scheduled + } if (mappedValue) { resolvedNode = mappedValue; + this.pendingUpdates.add(node.id); } } else { resolvedNode = null; @@ -161,19 +167,26 @@ export class AsyncNodePluginPlugin implements AsyncNodeViewPlugin { const newNode = resolvedNode || node; if (!resolvedNode && node?.type === NodeType.Async) { queueMicrotask(async () => { - const result = await this.basePlugin?.hooks.onAsyncNode.call(node); - const parsedNode = - options.parseNode && result - ? options.parseNode(result) - : undefined; - - this.resolvedMapping.set(node.id, parsedNode ? parsedNode : node); - view.updateAsync(); + if (!this.basePlugin) { + return; + } + this.basePlugin?.hooks.onAsyncNode.call(node).then((result) => { + const parsedNode = + options.parseNode && result + ? options.parseNode(result) + : undefined; + + if (parsedNode) { + this.resolvedMapping.set(node.id, parsedNode); + view.updateAsync(); + console.log("pending updates--", this.pendingUpdates.size); + this.pendingUpdates.delete(node.id); + } + }); }); return node; } - return newNode; }); });