Example asset and transform for async content streaming #575
Conversation
|
/canary |
cehan-Chloe
force-pushed
the
async-streaming-core
branch
from
8070a39 to
964cc11
Compare
|
|
||
| expect(result.type).toBe(NodeType.Value); | ||
| expect(result.children?.[0]?.value.type).toBe("asset"); | ||
| expect(result.children?.[0]?.value.value.id).toBe("asset"); |
There was a problem hiding this comment.
nit: change the id so failure is more clear
| @@ -36,7 +43,7 @@ export class Builder { | |||
| * @param values - the value or applicability nodes to put in the multinode | |||
| @@ -36,7 +43,7 @@ export class Builder { | |||
| * @param values - the value or applicability nodes to put in the multinode | |||
| */ | |||
| static multiNode( | |||
| ...values: (Node.Value | Node.Applicability)[] | |||
| ...values: (Node.Value | Node.Applicability | Node.Asset | Node.Async)[] | |||
There was a problem hiding this comment.
is this builder tested
core/player/src/view/parser/types.ts
Outdated
| @@ -116,6 +116,10 @@ export declare namespace Node { | |||
| id: string; | |||
| /** The value representing the node */ | |||
| value: Node; | |||
| /** | |||
| * Should this list be flattened in resolver | |||
There was a problem hiding this comment.
don't think async always guaranteed to be a list
There was a problem hiding this comment.
changed it to Should the new content streamed in be flattened during resolving?
|
|
||
| # ChatMessage Asset | ||
|
|
||
| The `ChatMessage` asset is used as an example of async node. |
There was a problem hiding this comment.
semantics, the ChatMessage itself isn't an async node, but it's an example of an asset that supports further async loading of future assets
| @@ -17,7 +17,7 @@ export function runTransform< | |||
| TransformedAssetType extends Asset = BaseAssetType, | |||
| >( | |||
| type: string, | |||
| transform: TransformFunction<BaseAssetType, TransformedAssetType>, | |||
| transform: TransformFunctions, | |||
There was a problem hiding this comment.
i might be missing something, what's this change
There was a problem hiding this comment.
before, this util was only testing TransformFunction which is afterResolve. but in chat-message we test beforeResolve as well and it's the same way to register it in AssetTransformPlugin so i changed to TransformFunctions interface for both before and after resolve
| let updateNumber = 0; | ||
|
|
||
| const player = new Player({ | ||
| plugins: [plugin, new ReferenceAssetsPlugin()], |
There was a problem hiding this comment.
nit: just group this together with the async one, pass in an array
| let view = (player.getState() as InProgressState).controllers.view | ||
| .currentView?.lastUpdate; | ||
|
|
||
| expect(view).toBeDefined(); |
There was a problem hiding this comment.
hmmm where's the first textview
There was a problem hiding this comment.
here checked the length of view values is 1 which is the first textview. line 647 tested the type. i can move it here to make it more clear.
| let view = (player.getState() as InProgressState).controllers.view | ||
| .currentView?.lastUpdate; | ||
|
|
||
| expect(view).toBeDefined(); |
There was a problem hiding this comment.
same here, how are we suddenly jumping from nothing to 2 texts
| expect(view?.values[0].asset.value).toBe("Hello World!"); | ||
| expect(view?.values[1].asset.type).toBe("text"); | ||
| expect(view?.values[1].asset.value).toBe("async content"); | ||
| }); |
There was a problem hiding this comment.
are you able to add a test that builds on top of this, that resolves the second chat-message async content as well
Change Type (required)
Indicate the type of change your pull request is:
patchminormajorN/ADoes your PR have any documentation updates?
Release Notes
Player streaming enhancement
chat-messageasset inReferenceAssetsPluginas async streaming exampleAsyncNodePluginto generate async asset transformInstead of adding
{async: true}in content, replace it with a special type of asset, eg.chat-messageand write a transform function.