-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ability to remove a resolved async node #404
Changes from 11 commits
50591e0
12f9d46
9e27f2d
ac7fec1
a198f1f
fc73d00
b95b53d
49a4d1d
db383d8
6764f0d
43e0edb
0030479
5c556c9
3bb57e1
c0d9dff
8403522
6cd0a7f
428a217
44f5bca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import { expect, test } from "vitest"; | ||
import { expect, test, vi } from "vitest"; | ||
import { Node, InProgressState, ViewInstance } from "@player-ui/player"; | ||
import { Player } from "@player-ui/player"; | ||
import { waitFor } from "@testing-library/react"; | ||
|
@@ -44,11 +44,20 @@ const asyncNodeTest = async (resolvedValue: any) => { | |
|
||
let deferredResolve: ((value: any) => void) | undefined; | ||
|
||
plugin.hooks.onAsyncNode.tap("test", async (node: Node.Async) => { | ||
return new Promise((resolve) => { | ||
deferredResolve = resolve; // Promise would be resolved only once | ||
}); | ||
}); | ||
let updateContent: any; | ||
|
||
plugin.hooks.onAsyncNode.tap( | ||
"test", | ||
async (node: Node.Async, update: (content: any) => void) => { | ||
const result = new Promise((resolve) => { | ||
deferredResolve = resolve; // Promise would be resolved only once | ||
}); | ||
|
||
updateContent = update; | ||
// Return the result to follow the same mechanism as before | ||
return result; | ||
}, | ||
); | ||
|
||
let updateNumber = 0; | ||
|
||
|
@@ -84,7 +93,7 @@ const asyncNodeTest = async (resolvedValue: any) => { | |
} | ||
|
||
await waitFor(() => { | ||
expect(updateNumber).toBe(2); | ||
expect(updateNumber).toBe(1); | ||
}); | ||
|
||
view = (player.getState() as InProgressState).controllers.view.currentView | ||
|
@@ -93,10 +102,14 @@ const asyncNodeTest = async (resolvedValue: any) => { | |
expect(view?.actions[0].asset.type).toBe("action"); | ||
expect(view?.actions.length).toBe(1); | ||
|
||
viewInstance?.update(); | ||
// Consumer responds with null/undefined | ||
if (deferredResolve) { | ||
updateContent(resolvedValue); | ||
} | ||
|
||
//Even after an 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(1); | ||
}); | ||
|
||
view = (player.getState() as InProgressState).controllers.view.currentView | ||
|
@@ -114,6 +127,90 @@ test("should return current node view when the resolved node is undefined", asyn | |
await asyncNodeTest(undefined); | ||
}); | ||
|
||
test("can handle multiple updates through callback mechanism", async () => { | ||
const plugin = new AsyncNodePlugin({ | ||
plugins: [new AsyncNodePluginPlugin()], | ||
}); | ||
|
||
let deferredResolve: ((value: any) => void) | undefined; | ||
|
||
let updateContent: any; | ||
|
||
plugin.hooks.onAsyncNode.tap( | ||
"test", | ||
async (node: Node.Async, update: (content: any) => void) => { | ||
const result = new Promise((resolve) => { | ||
deferredResolve = resolve; // Promise would be resolved only once | ||
}); | ||
|
||
updateContent = update; | ||
// Return the result to follow the same mechanism as before | ||
return result; | ||
}, | ||
); | ||
|
||
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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this not checking for something out of bounds? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean I think we shouldn't check for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Anyhow view?.actions[2] would be undefined , but yes as you said it makes no sense checking for this at this point of time as view?.actions[2] is not expected at all. Removed it |
||
|
||
if (deferredResolve) { | ||
updateContent(null); | ||
} | ||
|
||
await waitFor(() => { | ||
expect(updateNumber).toBe(3); | ||
}); | ||
|
||
view = (player.getState() as InProgressState).controllers.view.currentView | ||
?.lastUpdate; | ||
|
||
expect(view?.actions[0].asset.type).toBe("action"); | ||
expect(view?.actions[1]).toBeUndefined(); | ||
}); | ||
|
||
test("replaces async nodes with provided node", async () => { | ||
const plugin = new AsyncNodePlugin({ | ||
plugins: [new AsyncNodePluginPlugin()], | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍