From 45fd22629d9f6375f58b5f30e3bc8ca645f80ca7 Mon Sep 17 00:00:00 2001 From: Thomas Marmer Date: Fri, 22 Aug 2025 14:57:53 -0400 Subject: [PATCH 1/4] update createAsyncTransform to allow for different node types --- core/player/src/view/builder/index.ts | 6 +- .../__tests__/createAsyncTransform.test.ts | 220 +++++++++++++++++- .../core/src/createAsyncTransform.ts | 18 +- .../__tests__/requiresAssetWrapper.test.ts | 63 +++++ plugins/async-node/core/src/utils/index.ts | 1 + .../core/src/utils/requiresAssetWrapper.ts | 14 ++ 6 files changed, 312 insertions(+), 10 deletions(-) create mode 100644 plugins/async-node/core/src/utils/__tests__/requiresAssetWrapper.test.ts create mode 100644 plugins/async-node/core/src/utils/requiresAssetWrapper.ts diff --git a/core/player/src/view/builder/index.ts b/core/player/src/view/builder/index.ts index 109070cca..ba7b0a03d 100644 --- a/core/player/src/view/builder/index.ts +++ b/core/player/src/view/builder/index.ts @@ -39,11 +39,9 @@ export class Builder { * Creates a multiNode and associates the multiNode as the parent * of all the value nodes * - * @param values - the value, applicability or async nodes to put in the multinode + * @param values - the nodes to put in the multinode */ - static multiNode( - ...values: (Node.Value | Node.Applicability | Node.Async)[] - ): Node.MultiNode { + static multiNode(...values: Node.Node[]): Node.MultiNode { const m: Node.MultiNode = { type: NodeType.MultiNode, override: true, diff --git a/plugins/async-node/core/src/__tests__/createAsyncTransform.test.ts b/plugins/async-node/core/src/__tests__/createAsyncTransform.test.ts index d4c0b2d0a..524c64b70 100644 --- a/plugins/async-node/core/src/__tests__/createAsyncTransform.test.ts +++ b/plugins/async-node/core/src/__tests__/createAsyncTransform.test.ts @@ -1,6 +1,18 @@ -import { describe, it, expect, beforeEach } from "vitest"; +import { describe, it, expect, beforeEach, vi } from "vitest"; import { createAsyncTransform } from ".."; import { Builder, NodeType, Node } from "@player-ui/player"; +import { + extractNodeFromPath, + requiresAssetWrapper, + traverseAndReplace, + unwrapAsset, +} from "../utils"; + +vi.mock("../utils"); + +beforeEach(() => { + vi.mocked(requiresAssetWrapper).mockReturnValue(true); +}); describe("createAsyncTransform", () => { const asset = Builder.asset({ @@ -128,6 +140,198 @@ describe("createAsyncTransform", () => { }); }); + describe("getNestedAsset - different node types", () => { + it("should add the async node to an existing multi node", () => { + vi.mocked(requiresAssetWrapper).mockReturnValue(false); + const nodeIdFn = vi.fn(); + nodeIdFn.mockReturnValue("async-node"); + + const nestedAssetFn = vi.fn(); + const nestedAsset: Node.MultiNode = { + type: NodeType.MultiNode, + values: [ + { + type: NodeType.Value, + value: undefined, + children: [ + { + path: ["asset"], + value: { + type: NodeType.Asset, + value: { + type: "text", + id: "first-asset", + }, + }, + }, + ], + }, + { + type: NodeType.Value, + value: undefined, + children: [ + { + path: ["asset"], + value: { + type: NodeType.Asset, + value: { + type: "text", + id: "second-asset", + }, + }, + }, + ], + }, + ], + }; + nestedAssetFn.mockReturnValue(nestedAsset); + + const transform = createAsyncTransform({ + transformAssetType: "chat-message", + wrapperAssetType: "collection", + flatten: false, + path: ["array"], + getAsyncNodeId: nodeIdFn, + getNestedAsset: nestedAssetFn, + }); + + const result = transform(asset, {} as any, {} as any); + + expect(result).toStrictEqual({ + type: NodeType.Asset, + children: [ + { + path: ["array"], + value: { + type: NodeType.MultiNode, + override: true, + parent: expect.anything(), + values: [ + { + parent: expect.anything(), + type: NodeType.Value, + value: undefined, + children: [ + { + path: ["asset"], + value: { + type: NodeType.Asset, + value: { + type: "text", + id: "first-asset", + }, + }, + }, + ], + }, + { + parent: expect.anything(), + type: NodeType.Value, + value: undefined, + children: [ + { + path: ["asset"], + value: { + type: NodeType.Asset, + value: { + type: "text", + id: "second-asset", + }, + }, + }, + ], + }, + { + parent: expect.anything(), + type: NodeType.Async, + flatten: false, + onValueReceived: undefined, + id: "async-node", + value: { + type: NodeType.Value, + value: { + id: "async-node", + }, + }, + }, + ], + }, + }, + ], + value: { + id: "collection-async-node", + type: "collection", + }, + }); + }); + + it("should default to adding the node as-is", () => { + vi.mocked(requiresAssetWrapper).mockReturnValue(false); + const nodeIdFn = vi.fn(); + nodeIdFn.mockReturnValue("async-node"); + + const nestedAssetFn = vi.fn(); + const nestedAsset: Node.Value = { + type: NodeType.Value, + value: { + prop: "value", + }, + }; + nestedAssetFn.mockReturnValue(nestedAsset); + + const transform = createAsyncTransform({ + transformAssetType: "chat-message", + wrapperAssetType: "collection", + flatten: false, + path: ["array"], + getAsyncNodeId: nodeIdFn, + getNestedAsset: nestedAssetFn, + }); + + const result = transform(asset, {} as any, {} as any); + + expect(result).toStrictEqual({ + type: NodeType.Asset, + children: [ + { + path: ["array"], + value: { + type: NodeType.MultiNode, + override: true, + parent: expect.anything(), + values: [ + { + parent: expect.anything(), + type: NodeType.Value, + value: { + prop: "value", + }, + }, + { + parent: expect.anything(), + type: NodeType.Async, + flatten: false, + onValueReceived: undefined, + id: "async-node", + value: { + type: NodeType.Value, + value: { + id: "async-node", + }, + }, + }, + ], + }, + }, + ], + value: { + id: "collection-async-node", + type: "collection", + }, + }); + }); + }); + describe("onValueReceived callback setup", () => { let transformedAsset: Node.Node; let onValueReceivedFuncion: ((node: Node.Node) => Node.Node) | undefined; @@ -188,7 +392,19 @@ describe("createAsyncTransform", () => { expect(onValueReceivedFuncion).toBeDefined(); }); - it("should add an onValueReceivedFunction that transforms the result into a chained asset on the same type into a multi-node", () => { + it("should use traverseAndReplace as the onValueReceived callback", async () => { + const actualImplementation = + await vi.importActual("../utils"); + + vi.mocked(traverseAndReplace).mockImplementation( + actualImplementation.traverseAndReplace, + ); + vi.mocked(unwrapAsset).mockImplementation( + actualImplementation.unwrapAsset, + ); + vi.mocked(extractNodeFromPath).mockImplementation( + actualImplementation.extractNodeFromPath, + ); const result = onValueReceivedFuncion?.(asset); expect(result).toStrictEqual({ override: true, diff --git a/plugins/async-node/core/src/createAsyncTransform.ts b/plugins/async-node/core/src/createAsyncTransform.ts index 1fbd9d5fe..75fd1d95a 100644 --- a/plugins/async-node/core/src/createAsyncTransform.ts +++ b/plugins/async-node/core/src/createAsyncTransform.ts @@ -4,7 +4,12 @@ import { Node, NodeType, } from "@player-ui/player"; -import { extractNodeFromPath, traverseAndReplace, unwrapAsset } from "./utils"; +import { + extractNodeFromPath, + requiresAssetWrapper, + traverseAndReplace, + unwrapAsset, +} from "./utils"; export type AsyncTransformOptions = { /** Whether or not to flatten the results into its container. Defaults to true */ @@ -69,10 +74,15 @@ export const createAsyncTransform = ( const asyncNode = Builder.asyncNode(id, flatten, replaceFunction); let multiNode: Node.MultiNode | undefined; - if (asset) { - const assetNode = Builder.assetWrapper(asset); - multiNode = Builder.multiNode(assetNode, asyncNode); + if (requiresAssetWrapper(asset)) { + const assetWrappedNode = Builder.assetWrapper(asset); + multiNode = Builder.multiNode(assetWrappedNode, asyncNode); + } else if (asset.type === NodeType.MultiNode) { + multiNode = Builder.multiNode(...asset.values, asyncNode); + } else { + multiNode = Builder.multiNode(asset, asyncNode); + } } else { multiNode = Builder.multiNode(asyncNode); } diff --git a/plugins/async-node/core/src/utils/__tests__/requiresAssetWrapper.test.ts b/plugins/async-node/core/src/utils/__tests__/requiresAssetWrapper.test.ts new file mode 100644 index 000000000..1600f5b84 --- /dev/null +++ b/plugins/async-node/core/src/utils/__tests__/requiresAssetWrapper.test.ts @@ -0,0 +1,63 @@ +import { NodeType, Node } from "@player-ui/player"; +import { describe, expect, it } from "vitest"; +import { requiresAssetWrapper } from "../requiresAssetWrapper"; + +describe("requiresAssetWrapper", () => { + it("should return true for asset nodes", () => { + const node: Node.Asset = { + type: NodeType.Asset, + value: { + type: "text", + id: "id", + }, + }; + + const result = requiresAssetWrapper(node); + + expect(result).toBe(true); + }); + + it("should return true for applicability nodes containing asset nodes", () => { + const node: Node.Applicability = { + type: NodeType.Applicability, + expression: "", + value: { + type: NodeType.Asset, + value: { + type: "text", + id: "id", + }, + }, + }; + + const result = requiresAssetWrapper(node); + + expect(result).toBe(true); + }); + + it("should return false for non-asset or non-applicability nodes", () => { + const node: Node.Value = { + type: NodeType.Value, + value: {}, + }; + + const result = requiresAssetWrapper(node); + + expect(result).toBe(false); + }); + + it("should return false for applicability nodes that do not contain an asset node", () => { + const node: Node.Applicability = { + type: NodeType.Applicability, + expression: "", + value: { + type: NodeType.Value, + value: {}, + }, + }; + + const result = requiresAssetWrapper(node); + + expect(result).toBe(false); + }); +}); diff --git a/plugins/async-node/core/src/utils/index.ts b/plugins/async-node/core/src/utils/index.ts index b97c76388..98139ad44 100644 --- a/plugins/async-node/core/src/utils/index.ts +++ b/plugins/async-node/core/src/utils/index.ts @@ -1,3 +1,4 @@ export * from "./extractNodeFromPath"; export * from "./traverseAndReplace"; export * from "./unwrapAsset"; +export * from "./requiresAssetWrapper"; diff --git a/plugins/async-node/core/src/utils/requiresAssetWrapper.ts b/plugins/async-node/core/src/utils/requiresAssetWrapper.ts new file mode 100644 index 000000000..f8907da67 --- /dev/null +++ b/plugins/async-node/core/src/utils/requiresAssetWrapper.ts @@ -0,0 +1,14 @@ +import { NodeType } from "@player-ui/player"; +import type { Node } from "@player-ui/player"; + +export const requiresAssetWrapper = (node: Node.Node): boolean => { + if (node.type === NodeType.Asset) { + return true; + } + + if (node.type !== NodeType.Applicability) { + return false; + } + + return node.value.type === NodeType.Asset; +}; From 52995f770c930cecb75a6577382c6be67ef849dc Mon Sep 17 00:00:00 2001 From: Thomas Marmer Date: Mon, 25 Aug 2025 12:41:06 -0400 Subject: [PATCH 2/4] undo type change to Builder.multiNode --- core/player/src/view/builder/index.ts | 6 ++++-- core/player/src/view/resolver/__tests__/index.test.ts | 4 ++-- plugins/async-node/core/src/createAsyncTransform.ts | 11 +++++++++-- .../async-node/core/src/utils/requiresAssetWrapper.ts | 4 +++- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/core/player/src/view/builder/index.ts b/core/player/src/view/builder/index.ts index ba7b0a03d..109070cca 100644 --- a/core/player/src/view/builder/index.ts +++ b/core/player/src/view/builder/index.ts @@ -39,9 +39,11 @@ export class Builder { * Creates a multiNode and associates the multiNode as the parent * of all the value nodes * - * @param values - the nodes to put in the multinode + * @param values - the value, applicability or async nodes to put in the multinode */ - static multiNode(...values: Node.Node[]): Node.MultiNode { + static multiNode( + ...values: (Node.Value | Node.Applicability | Node.Async)[] + ): Node.MultiNode { const m: Node.MultiNode = { type: NodeType.MultiNode, override: true, diff --git a/core/player/src/view/resolver/__tests__/index.test.ts b/core/player/src/view/resolver/__tests__/index.test.ts index 81565c4cf..831ad2a47 100644 --- a/core/player/src/view/resolver/__tests__/index.test.ts +++ b/core/player/src/view/resolver/__tests__/index.test.ts @@ -58,7 +58,7 @@ describe("Async Node Resolution", () => { }; }); - it("should", () => { + it("should clear the cache for the async node and its parent when it is updated", () => { const beforeResolveFunction = vi.fn((node: Node.Node | null) => node); const resolver = new Resolver(simpleViewWithAsync, resolverOptions); @@ -106,7 +106,7 @@ describe("Async Node Resolution", () => { ); }); - it("should also", () => { + it("should clear the cache for anything with a matching async node in its resolved list on update", () => { const beforeResolveFunction = vi.fn((node: Node.Node | null) => { // Add asyncNodesResolved to view to test tracking and invalidation of just the view. if (node?.type === NodeType.View) { diff --git a/plugins/async-node/core/src/createAsyncTransform.ts b/plugins/async-node/core/src/createAsyncTransform.ts index 75fd1d95a..c554a0f58 100644 --- a/plugins/async-node/core/src/createAsyncTransform.ts +++ b/plugins/async-node/core/src/createAsyncTransform.ts @@ -21,7 +21,14 @@ export type AsyncTransformOptions = { /** The asset type that will contain the async content. */ wrapperAssetType: string; /** Function to get any nested asset that will need to be extracted and kept when creating the wrapper asset. */ - getNestedAsset?: (node: Node.ViewOrAsset) => Node.Node | undefined; + getNestedAsset?: ( + node: Node.ViewOrAsset, + ) => + | Node.Asset + | Node.Value + | Node.Applicability + | Node.MultiNode + | undefined; /** Function to get the id for the async node being generated. Defaults to creating an id with the format of async- */ getAsyncNodeId?: (node: Node.ViewOrAsset) => string; }; @@ -79,7 +86,7 @@ export const createAsyncTransform = ( const assetWrappedNode = Builder.assetWrapper(asset); multiNode = Builder.multiNode(assetWrappedNode, asyncNode); } else if (asset.type === NodeType.MultiNode) { - multiNode = Builder.multiNode(...asset.values, asyncNode); + multiNode = Builder.multiNode(...(asset.values as any[]), asyncNode); } else { multiNode = Builder.multiNode(asset, asyncNode); } diff --git a/plugins/async-node/core/src/utils/requiresAssetWrapper.ts b/plugins/async-node/core/src/utils/requiresAssetWrapper.ts index f8907da67..5bd1546be 100644 --- a/plugins/async-node/core/src/utils/requiresAssetWrapper.ts +++ b/plugins/async-node/core/src/utils/requiresAssetWrapper.ts @@ -1,7 +1,9 @@ import { NodeType } from "@player-ui/player"; import type { Node } from "@player-ui/player"; -export const requiresAssetWrapper = (node: Node.Node): boolean => { +export const requiresAssetWrapper = ( + node: Node.Node, +): node is Node.Applicability | Node.Asset => { if (node.type === NodeType.Asset) { return true; } From 158eeb5fd3ecc88b4a103201562a8f3f5c6d8205 Mon Sep 17 00:00:00 2001 From: Thomas Marmer Date: Mon, 25 Aug 2025 13:19:21 -0400 Subject: [PATCH 3/4] fix type build issues on reference assets --- plugins/async-node/core/src/createAsyncTransform.ts | 11 ++--------- .../async-node/core/src/utils/requiresAssetWrapper.ts | 4 +--- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/plugins/async-node/core/src/createAsyncTransform.ts b/plugins/async-node/core/src/createAsyncTransform.ts index c554a0f58..45bf49093 100644 --- a/plugins/async-node/core/src/createAsyncTransform.ts +++ b/plugins/async-node/core/src/createAsyncTransform.ts @@ -21,14 +21,7 @@ export type AsyncTransformOptions = { /** The asset type that will contain the async content. */ wrapperAssetType: string; /** Function to get any nested asset that will need to be extracted and kept when creating the wrapper asset. */ - getNestedAsset?: ( - node: Node.ViewOrAsset, - ) => - | Node.Asset - | Node.Value - | Node.Applicability - | Node.MultiNode - | undefined; + getNestedAsset?: (node: Node.ViewOrAsset) => Node.Node | undefined; /** Function to get the id for the async node being generated. Defaults to creating an id with the format of async- */ getAsyncNodeId?: (node: Node.ViewOrAsset) => string; }; @@ -88,7 +81,7 @@ export const createAsyncTransform = ( } else if (asset.type === NodeType.MultiNode) { multiNode = Builder.multiNode(...(asset.values as any[]), asyncNode); } else { - multiNode = Builder.multiNode(asset, asyncNode); + multiNode = Builder.multiNode(asset as any, asyncNode); } } else { multiNode = Builder.multiNode(asyncNode); diff --git a/plugins/async-node/core/src/utils/requiresAssetWrapper.ts b/plugins/async-node/core/src/utils/requiresAssetWrapper.ts index 5bd1546be..f8907da67 100644 --- a/plugins/async-node/core/src/utils/requiresAssetWrapper.ts +++ b/plugins/async-node/core/src/utils/requiresAssetWrapper.ts @@ -1,9 +1,7 @@ import { NodeType } from "@player-ui/player"; import type { Node } from "@player-ui/player"; -export const requiresAssetWrapper = ( - node: Node.Node, -): node is Node.Applicability | Node.Asset => { +export const requiresAssetWrapper = (node: Node.Node): boolean => { if (node.type === NodeType.Asset) { return true; } From 181e4ff645362e5d3064339e609dc932647129b1 Mon Sep 17 00:00:00 2001 From: Thomas Marmer Date: Thu, 28 Aug 2025 13:22:23 -0400 Subject: [PATCH 4/4] remove mocks from async transform tests --- .../__tests__/createAsyncTransform.test.ts | 28 +------------------ 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/plugins/async-node/core/src/__tests__/createAsyncTransform.test.ts b/plugins/async-node/core/src/__tests__/createAsyncTransform.test.ts index 524c64b70..52f011acf 100644 --- a/plugins/async-node/core/src/__tests__/createAsyncTransform.test.ts +++ b/plugins/async-node/core/src/__tests__/createAsyncTransform.test.ts @@ -1,18 +1,6 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { createAsyncTransform } from ".."; import { Builder, NodeType, Node } from "@player-ui/player"; -import { - extractNodeFromPath, - requiresAssetWrapper, - traverseAndReplace, - unwrapAsset, -} from "../utils"; - -vi.mock("../utils"); - -beforeEach(() => { - vi.mocked(requiresAssetWrapper).mockReturnValue(true); -}); describe("createAsyncTransform", () => { const asset = Builder.asset({ @@ -142,7 +130,6 @@ describe("createAsyncTransform", () => { describe("getNestedAsset - different node types", () => { it("should add the async node to an existing multi node", () => { - vi.mocked(requiresAssetWrapper).mockReturnValue(false); const nodeIdFn = vi.fn(); nodeIdFn.mockReturnValue("async-node"); @@ -266,7 +253,6 @@ describe("createAsyncTransform", () => { }); it("should default to adding the node as-is", () => { - vi.mocked(requiresAssetWrapper).mockReturnValue(false); const nodeIdFn = vi.fn(); nodeIdFn.mockReturnValue("async-node"); @@ -392,19 +378,7 @@ describe("createAsyncTransform", () => { expect(onValueReceivedFuncion).toBeDefined(); }); - it("should use traverseAndReplace as the onValueReceived callback", async () => { - const actualImplementation = - await vi.importActual("../utils"); - - vi.mocked(traverseAndReplace).mockImplementation( - actualImplementation.traverseAndReplace, - ); - vi.mocked(unwrapAsset).mockImplementation( - actualImplementation.unwrapAsset, - ); - vi.mocked(extractNodeFromPath).mockImplementation( - actualImplementation.extractNodeFromPath, - ); + it("should use traverseAndReplace as the onValueReceived callback", () => { const result = onValueReceivedFuncion?.(asset); expect(result).toStrictEqual({ override: true,