Skip to content
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

Merged
merged 19 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
50591e0
Rebased onto main branch for ability to remove a resolved async node
sakuntala-motukuri Jul 16, 2024
12f9d46
Merge branch 'main' into bazel-6-Ability-to-remove-a-resolved-async-node
sakuntala-motukuri Jul 16, 2024
9e27f2d
Updated code with callback mechanism and updated respective test cases
sakuntala-motukuri Jul 18, 2024
ac7fec1
Merge branch 'main' into bazel-6-Ability-to-remove-a-resolved-async-node
sakuntala-motukuri Jul 18, 2024
a198f1f
Fixed PR review comments
sakuntala-motukuri Jul 18, 2024
fc73d00
Fixed PR review comments second time
sakuntala-motukuri Jul 18, 2024
b95b53d
updated test to be in a waitfor
sakuntala-motukuri Jul 18, 2024
49a4d1d
removed AsyncNodePlugin initialisation to satiate the test accordingly
sakuntala-motukuri Jul 18, 2024
db383d8
removed unnecessary code and tests
sakuntala-motukuri Jul 18, 2024
6764f0d
Updated docs for async-node.mdx
sakuntala-motukuri Jul 19, 2024
43e0edb
Updated suggested changes
sakuntala-motukuri Jul 19, 2024
0030479
Fixed additional review comments
sakuntala-motukuri Jul 22, 2024
5c556c9
Merge branch 'main' into bazel-6-Ability-to-remove-a-resolved-async-node
sakuntala-motukuri Jul 22, 2024
3bb57e1
Code cleanup as per ask by Tony
sakuntala-motukuri Jul 22, 2024
c0d9dff
Merge branch 'main' into bazel-6-Ability-to-remove-a-resolved-async-node
sakuntala-motukuri Jul 22, 2024
8403522
Updated naming convention
sakuntala-motukuri Jul 22, 2024
6cd0a7f
view instance was resolving to null due to which the tests were faili…
sakuntala-motukuri Jul 23, 2024
428a217
Merge branch 'main' into bazel-6-Ability-to-remove-a-resolved-async-node
sakuntala-motukuri Jul 23, 2024
44f5bca
bind
brocollie08 Jul 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions docs/site/pages/plugins/async-node.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ In the example below, node with the id "some-async-node" will not be rendered on

The `AsyncNodePlugin` exposes an `onAsyncNode` hook on all platforms. The `onAsyncNode` hook will be invoked with the current node when the plugin is available and an `AsyncNode` is detected during the resolve process. The node used to call the hook with could contain metadata according to content spec.

User should tap into the `onAsyncNode` hook to examine the node's metadata before making a decision on what to replace the async node with. The return could be a single asset node or an array of asset nodes.
User should tap into the `onAsyncNode` hook to examine the node's metadata before making a decision on what to replace the async node with. The return could be a single asset node or an array of asset nodes, or the return could be even a null|undefined if the async node is no longer relevant.

Returning a value in the above context enables uses cases where the async node only needs to be resolved once. For use cases where the async node needs to be updated multiple times, the onAsyncNode hook provides a second callback argument that can be used to update the value multiple times. For example, if the async node is used to represent some placeholder for toasts, or notifications, the async node handler could initially resolve with some content, and then update with null after some time to remove those views.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


### Continuous Streaming

Expand All @@ -42,7 +43,8 @@ This means there must be a constant renewal of new `AsyncNode`s after the previo

## Usage

The AsyncNodePlugin itself accepts a list of plugins and should take the AsyncNodePluginPlugin to be passed in. The AsyncNodePluginPlugin also comes from the `'@player-ui/async-node-plugin'` and contains the resolver and parser functionality
The `AsyncNodePlugin` itself accepts an options object with a `plugins` array, enabling the integration of multiple view plugins for extended functionality.
The `AsyncNodePluginPlugin` is provided as a default way of handling asset-async nodes, it is just one handler for one possible way of using async nodes. If the default behavior does not align with the desired usage, users are able to provide their own implementation of the handler in the form of a plugin to be passed to the base `AsyncNodePlugin`. The `AsyncNodePluginPlugin` also comes from the `'@player-ui/async-node-plugin'` and contains the resolver and parser functionality.

<PlatformTabs>
<core>
Expand All @@ -63,6 +65,18 @@ asyncNodePlugin.hooks.onAsyncNode.tap('handleAsync', async (node: Node.Node) =>
// Determine what to return to be parsed into a concrete UI asset
});

// For use cases where the async node needs to be updated multiple times

asyncNodePlugin.hooks.onAsyncNode.tap("toast-provider", async (node: Node.Async, update: (content) => void) => {
...
// do some async task to get content
const toastContent = await makeToastFor(node.id);
// set timer for 5 seconds to remove the toast content from the view
setTimeout(() => update(null), 5000);
// uses same mechanism as before
return toastContent;
});

const player = new Player({
plugins: [
asyncNodePlugin
Expand Down Expand Up @@ -164,7 +178,7 @@ return .multiNode([

Note: the AsyncNode struct is already defined in the plugin with the `async` property defaulted to true so only `id` needs to be passed in

As a convenience to the user, the AsyncNodePlugin just takes a callback which has the content to be returned, this is provided to the plugin which calls the the `onAsyncNode` hook tap method
As a convenience to the user, the AsyncNodePlugin just takes a callback which has the content to be returned, this is provided to the plugin which calls the the `onAsyncNode` hook tap method. The return could be a single asset node or an array of asset nodes, or null if the async node is no longer relevant.

</ios>
<android>
Expand Down
114 changes: 105 additions & 9 deletions plugins/async-node/core/src/index.test.ts
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";
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -114,6 +127,89 @@ 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(updateNumber).toBe(2);

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()],
Expand Down
141 changes: 74 additions & 67 deletions plugins/async-node/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {
Parser,
ViewPlugin,
Resolver,
Resolve,
} from "@player-ui/player";
import { AsyncParallelBailHook } from "tapable-ts";
import queueMicrotask from "queue-microtask";
Expand All @@ -24,7 +25,7 @@ export interface AsyncNodeViewPlugin extends ViewPlugin {
/** Use this to tap into the async node plugin hooks */
applyPlugin: (asyncNodePlugin: AsyncNodePlugin) => void;

asyncNode: AsyncParallelBailHook<[Node.Async], any>;
asyncNode: AsyncParallelBailHook<[Node.Async, (result: any) => void], any>;
}

/**
Expand All @@ -44,7 +45,10 @@ export class AsyncNodePlugin implements PlayerPlugin {
}

public readonly hooks = {
onAsyncNode: new AsyncParallelBailHook<[Node.Async], any>(),
onAsyncNode: new AsyncParallelBailHook<
[Node.Async, (result: any) => void],
any
>(),
};

name = "AsyncNode";
Expand All @@ -61,7 +65,10 @@ export class AsyncNodePlugin implements PlayerPlugin {
}

export class AsyncNodePluginPlugin implements AsyncNodeViewPlugin {
public asyncNode = new AsyncParallelBailHook<[Node.Async], any>();
public asyncNode = new AsyncParallelBailHook<
[Node.Async, (result: any) => void],
any
>();
private basePlugin: AsyncNodePlugin | undefined;

name = "AsyncNode";
Expand All @@ -70,6 +77,67 @@ export class AsyncNodePluginPlugin implements AsyncNodeViewPlugin {

private currentView: ViewInstance | undefined;

/**
* Updates the node asynchronously based on the result provided.
* This method is responsible for handling the update logic of asynchronous nodes.
* It checks if the node needs to be updated based on the new result and updates the mapping accordingly.
* If an update is necessary, it triggers an asynchronous update on the view.
* @param node The asynchronous node that might be updated.
* @param result The result obtained from resolving the async node. This could be any data structure or value.
* @param options Options provided for node resolution, including a potential parseNode function to process the result.
* @param view The view instance where the node resides. This can be undefined if the view is not currently active.
*/
private handleAsyncUpdate(
node: Node.Async,
result: any,
options: Resolve.NodeResolveOptions,
view: ViewInstance | undefined,
) {
const parsedNode =
options.parseNode && result ? options.parseNode(result) : undefined;

if (this.resolvedMapping.get(node.id) !== parsedNode) {
this.resolvedMapping.set(node.id, parsedNode ? parsedNode : node);
view?.updateAsync();
}
}

/**
* Handles the asynchronous API integration for resolving nodes.
* This method sets up a hook on the resolver's `beforeResolve` event to process async nodes.
* @param resolver The resolver instance to attach the hook to.
* @param view
*/
applyResolver(resolver: Resolver) {
resolver.hooks.beforeResolve.tap(this.name, (node, options) => {
let resolvedNode;
if (this.isAsync(node)) {
const mappedValue = this.resolvedMapping.get(node.id);
if (mappedValue) {
resolvedNode = mappedValue;
}
} else {
resolvedNode = null;
}

const newNode = resolvedNode || node;
if (!resolvedNode && node?.type === NodeType.Async) {
queueMicrotask(async () => {
const result = await this.basePlugin?.hooks.onAsyncNode.call(
node,
(result) => {
this.handleAsyncUpdate(node, result, options, this.currentView);
},
);
this.handleAsyncUpdate(node, result, options, this.currentView);
});

return node;
}
return newNode;
});
}

private isAsync(node: Node.Node | null): node is Node.Async {
return node?.type === NodeType.Async;
}
Expand Down Expand Up @@ -112,71 +180,10 @@ export class AsyncNodePluginPlugin implements AsyncNodeViewPlugin {
);
}

applyResolverHooks(resolver: Resolver) {
resolver.hooks.beforeResolve.tap(this.name, (node, options) => {
let resolvedNode;
if (this.isAsync(node)) {
const mappedValue = this.resolvedMapping.get(node.id);
if (mappedValue) {
resolvedNode = mappedValue;
}
} else {
resolvedNode = null;
}

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;

if (parsedNode) {
this.resolvedMapping.set(node.id, parsedNode);
this.currentView?.updateAsync();
}
});

return node;
}

return newNode;
});
}

apply(view: ViewInstance): void {
view.hooks.parser.tap("template", this.applyParser.bind(this));
view.hooks.resolver.tap("template", (resolver) => {
resolver.hooks.beforeResolve.tap(this.name, (node, options) => {
let resolvedNode;
if (this.isAsync(node)) {
const mappedValue = this.resolvedMapping.get(node.id);
if (mappedValue) {
resolvedNode = mappedValue;
}
} else {
resolvedNode = null;
}

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();
});

return node;
}

return newNode;
});
});
this.currentView = view;
view.hooks.parser.tap("async", this.applyParser.bind(this));
view.hooks.resolver.tap("async", this.applyResolver.bind(this));
}

applyPlugin(asyncNodePlugin: AsyncNodePlugin): void {
Expand Down