feat(devtools-core): Visualize TreeDataObject in Devtools #24885
Conversation
github-actions
bot
added
base: main
github-actions
bot
removed
area: dds
github-actions
bot
removed
the
area: framework
| public registerDataObject(props: DataObjectProps): void { | ||
| const { runtime, dataObject } = props; | ||
|
|
||
| const runtimeId = `${runtime.id}-${Math.random().toString(36).slice(2, 11)}`; |
There was a problem hiding this comment.
As far as I remember, we were assuming 1-runtime per 1-dataobject, thus the containers will be storing unqiue runtime.id.
However, I've been keep getting A as runtime.id over different PureDataObject.runtime. Still investigating why, but in the meantime assigned random number to bypass the issue.
There was a problem hiding this comment.
Sorry, just to clarify: you're saying that runtime.id is the string literal "^" for all of the data objects?
If so, then we may have a problem, and this approach may not work. We really need some globally unique identifier for this schema to work. Ideally, we could just get the ID of the container associated with the data object. Barring that, some other unique identifier to group data objects under.
There was a problem hiding this comment.
If the same runtime generates a bunch of dds. Ideally, don't we want them to be shown under a same tag? This will make the runtime id different all the time. Why can't we just ${runtime.id}?
There was a problem hiding this comment.
More precisely, I am using runtime.id here to distinguish between different PureDataObject not DDSs. However, for some reason, runtime.id of different DataObject (e.g., AppData, AppDataTwo, and etc.) are all throwing same id which throws an runtime error in the devtools side.
There was a problem hiding this comment.
Yes, because these two objects is generated by the same runtime. Ideally, shouldn't under this runtimeId there are two objects listed under the tag Data?
There was a problem hiding this comment.
I wonder if the "^" runtime ID is due to our use of internal test infrastructure. I would love to see what IDs we get once you've prototyped integration into Loop. I do see that PureDataObject.id just returns its this.runtime.id, so I suspect each data object will have its own unique runtime.
If that's case, a follow up question is whether or not that ID is globally unique, i.e. unique even across containers. My guess is that they likely must be unique within a container, but probably not across them. If they are globally unique, then we can always create a more customized UX that lists "Data Objects" instead of "Containers". If they're not globally unique, then we have a problem.
| * | ||
| * @alpha | ||
| */ | ||
| export interface DecomposedIContainer extends IEventProvider<IContainerEvents> { |
There was a problem hiding this comment.
How often does IContainer change? How do you detect when DecomposedIContainer goes out of sync with the IContainer API? It feels like you need something like a type test, but one that compares members and signatures for two different types.
There was a problem hiding this comment.
The package's APIs should still be expressed in terms of IContainer (I left some feedback to this effect elsewhere). In that model, this package will fail to build if they become out-of-sync.
packages/tools/devtools/devtools-core/src/DecomposedContainer.ts
Outdated
Show resolved
Hide resolved
packages/tools/devtools/devtools-core/src/DecomposedContainer.ts
Outdated
Show resolved
Hide resolved
| * Implementation of {@link DecomposedIContainer} that wraps an {@link IFluidDataStoreRuntime}. | ||
| * This class provides a bridge between {@link IFluidDataStoreRuntime} and the devtools system by exposing runtime properties and events as {@link IContainer} interfaces. | ||
| * | ||
| * @alpha |
There was a problem hiding this comment.
We shouldn't export this class, so it shouldn't need a release tag.
packages/tools/devtools/devtools-core/api-report/devtools-core.alpha.api.md
Outdated
Show resolved
Hide resolved
| @@ -281,15 +300,15 @@ export class ContainerDevtools implements IContainerDevtools, HasContainerKey { | |||
| [ConnectContainer.MessageType]: async (untypedMessage) => { | |||
| const message = untypedMessage as ConnectContainer.Message; | |||
| if (message.data.containerKey === this.containerKey) { | |||
| this.container.connect(); | |||
| this.container.connect?.(); | |||
There was a problem hiding this comment.
We'll need to make these behaviors more robust before merging. We should update things such that the UI doesn't display the connect/disconnect buttons when they are not available.
There was a problem hiding this comment.
I would recommend making this PR a draft. There is still a fair amount of work that needs to be done to this package and devtools-view to make this new infrastructure robust. I've left a handful of comments to this effect. Great proof of concept, but there is still a fair amount of work to be done to productionize it.
Co-authored-by: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com>
| @@ -94,6 +96,7 @@ declare type current_as_old_for_Interface_IDevtoolsLogger = requireAssignableTo< | |||
| * typeValidation.broken: | |||
| * "Interface_IFluidDevtools": {"forwardCompat": false} | |||
| */ | |||
| // @ts-expect-error compatibility expected to be broken | |||
There was a problem hiding this comment.
This backward incompatibility will break the office-bohemia because we recently check-in the "integrate devtools in loop" code change and that commit use registerContainerDevtools API.
| /** | ||
| * The data object to register with the Devtools. | ||
| */ | ||
| dataObject: PureDataObject; |
There was a problem hiding this comment.
TreeDataObject is @internal. Thus using PureDataObject for now.
|
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output |
NOTE
This PR prototypes a
devtools-coreAPI integration and explores its potential use cases.Background
This work originates from ADO 36175, which proposed creating an adapter in the
office-bohemiarepo to support per-Loop-Component (DataObject) visualization. However, implementing a fullIContaineradapter was not feasible, as the required properties for thedevtools-corevisualizer couldn't be fulfilled.Description
Rather than wrapping everything into an adapter, this PR takes the inverse approach. It introduces a new
DecomposedIContainerinterface and aDecomposedContainerclass that implements it. This class is a lightweight, strict subset of theIContainerinterface, exposing only the properties necessary for visualization.Thanks to the property and event overlap (
clientId,audience,connected,attached, etc.) withPureDataObject.runtime, we can effectively translate aPureDataObjectinto a decomposedIContainertype, enabling visualizer compatibility.