-
Notifications
You must be signed in to change notification settings - Fork 535
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
feat(devtools-core): Render ST's Visualizer to Include Root Field's Allowed Types #23573
base: main
Are you sure you want to change the base?
Changes from all commits
df3da17
fd50e40
cf42f7a
1e29755
5e96b73
9a6793a
d4c4901
4990a60
363ced9
dbf8f9b
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 |
---|---|---|
|
@@ -91,7 +91,7 @@ function createToolTipContents(schema: SharedTreeSchemaNode): VisualTreeNode { | |
} | ||
|
||
/** | ||
* Converts the visual representation from {@link visualizeSharedTreeNodeBySchema} to a visual tree compatible with the devtools-view. | ||
* Converts the visual representation from {@link visualizeInternalNodeBySchema} to a visual tree compatible with the devtools-view. | ||
* @param tree - the visual representation of the SharedTree. | ||
* @returns - the visual representation of type {@link VisualChildNode} | ||
*/ | ||
|
@@ -150,7 +150,7 @@ export function toVisualTree(tree: VisualSharedTreeNode): VisualChildNode { | |
/** | ||
* Concatenrate allowed types for `ObjectNodeStoredSchema` and `MapNodeStoredSchema`. | ||
*/ | ||
function concatenateTypes(fieldTypes: ReadonlySet<string>): string { | ||
export function concatenateTypes(fieldTypes: ReadonlySet<string>): string { | ||
return [...fieldTypes].join(" | "); | ||
} | ||
|
||
|
@@ -172,16 +172,14 @@ function getObjectAllowedTypes(schema: SimpleObjectNodeSchema): string { | |
* Returns the schema & fields of the node. | ||
*/ | ||
async function visualizeVerboseNodeFields( | ||
tree: VerboseTreeNode, | ||
treeFields: VerboseTree[] | Record<string, VerboseTree>, | ||
treeSchema: SimpleTreeSchema, | ||
visualizeChildData: VisualizeChildData, | ||
): Promise<Record<string, VisualSharedTreeNode>> { | ||
const treeFields = tree.fields; | ||
|
||
const fields: Record<string | number, VisualSharedTreeNode> = {}; | ||
|
||
for (const [fieldKey, childField] of Object.entries(treeFields)) { | ||
fields[fieldKey] = await visualizeSharedTreeNodeBySchema( | ||
fields[fieldKey] = await visualizeSharedTreeBySchema( | ||
childField, | ||
treeSchema, | ||
visualizeChildData, | ||
|
@@ -200,12 +198,16 @@ async function visualizeObjectNode( | |
treeSchema: SimpleTreeSchema, | ||
visualizeChildData: VisualizeChildData, | ||
): Promise<VisualSharedTreeNode> { | ||
const isRootField = treeSchema.allowedTypes.has(tree.type); | ||
|
||
return { | ||
schema: { | ||
schemaName: tree.type, | ||
allowedTypes: getObjectAllowedTypes(nodeSchema), | ||
allowedTypes: isRootField | ||
? concatenateTypes(treeSchema.allowedTypes) | ||
: getObjectAllowedTypes(nodeSchema), | ||
}, | ||
fields: await visualizeVerboseNodeFields(tree, treeSchema, visualizeChildData), | ||
fields: await visualizeVerboseNodeFields(tree.fields, treeSchema, visualizeChildData), | ||
kind: VisualSharedTreeNodeKind.InternalNode, | ||
}; | ||
} | ||
|
@@ -224,36 +226,25 @@ async function visualizeMapNode( | |
schemaName: tree.type, | ||
allowedTypes: `Record<string, ${concatenateTypes(nodeSchema.allowedTypes)}>`, | ||
}, | ||
fields: await visualizeVerboseNodeFields(tree, treeSchema, visualizeChildData), | ||
fields: await visualizeVerboseNodeFields(tree.fields, treeSchema, visualizeChildData), | ||
kind: VisualSharedTreeNodeKind.InternalNode, | ||
}; | ||
} | ||
|
||
/** | ||
* Main recursive helper function to create the visual representation of the SharedTree. | ||
* Processes tree nodes based on their schema type (e.g., ObjectNodeStoredSchema, MapNodeStoredSchema, LeafNodeStoredSchema), producing the visual representation for each type. | ||
* Helper function to create the visual representation of non-leaf SharedTree nodes. | ||
* Processes internal tree nodes based on their schema type (e.g., ObjectNodeStoredSchema, MapNodeStoredSchema, ArrayNodeStoredSchema), | ||
* producing the visual representation for each type. | ||
* | ||
* @see {@link https://fluidframework.com/docs/data-structures/tree/} for more information on the SharedTree schema. | ||
* | ||
* @remarks | ||
*/ | ||
export async function visualizeSharedTreeNodeBySchema( | ||
tree: VerboseTree, | ||
async function visualizeInternalNodeBySchema( | ||
tree: VerboseTreeNode, | ||
treeSchema: SimpleTreeSchema, | ||
visualizeChildData: VisualizeChildData, | ||
): Promise<VisualSharedTreeNode> { | ||
const sf = new SchemaFactory(undefined); | ||
if (Tree.is(tree, [sf.boolean, sf.null, sf.number, sf.handle, sf.string])) { | ||
const nodeSchema = Tree.schema(tree); | ||
return { | ||
schema: { | ||
schemaName: nodeSchema.identifier, | ||
}, | ||
value: await visualizeChildData(tree), | ||
kind: VisualSharedTreeNodeKind.LeafNode, | ||
}; | ||
} | ||
|
||
const schema = treeSchema.definitions.get(tree.type); | ||
if (schema === undefined) { | ||
throw new TypeError("Unrecognized schema type."); | ||
|
@@ -281,7 +272,7 @@ export async function visualizeSharedTreeNodeBySchema( | |
} | ||
|
||
for (let i = 0; i < children.length; i++) { | ||
fields[i] = await visualizeSharedTreeNodeBySchema( | ||
fields[i] = await visualizeSharedTreeBySchema( | ||
children[i], | ||
treeSchema, | ||
visualizeChildData, | ||
|
@@ -293,7 +284,7 @@ export async function visualizeSharedTreeNodeBySchema( | |
schemaName: tree.type, | ||
allowedTypes: concatenateTypes(schema.allowedTypes), | ||
}, | ||
fields: await visualizeVerboseNodeFields(tree, treeSchema, visualizeChildData), | ||
fields: await visualizeVerboseNodeFields(tree.fields, treeSchema, visualizeChildData), | ||
kind: VisualSharedTreeNodeKind.InternalNode, | ||
}; | ||
} | ||
|
@@ -302,3 +293,44 @@ export async function visualizeSharedTreeNodeBySchema( | |
} | ||
} | ||
} | ||
|
||
/** | ||
* Creates a visual representation of a SharedTree based on its schema. | ||
* @param tree - The {@link VerboseTree} to visualize | ||
* @param treeSchema - The schema that defines the structure and types of the tree | ||
* @param visualizeChildData - Callback function to visualize child node data | ||
* @returns A visual representation of the tree that includes schema information and node values | ||
* | ||
* @remarks | ||
* This function handles both leaf nodes (primitive values, handles) and internal nodes (objects, maps, arrays). | ||
* For leaf nodes, it creates a visual representation with the node's schema and value. | ||
* For internal nodes, it recursively processes the node's fields using {@link visualizeInternalNodeBySchema}. | ||
*/ | ||
export async function visualizeSharedTreeBySchema( | ||
tree: VerboseTree, | ||
treeSchema: SimpleTreeSchema, | ||
visualizeChildData: VisualizeChildData, | ||
isRootField?: boolean, | ||
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. Interested in your thoughts of this optional parameter. This is to display all the allowed types of the "first" rootfield when it is populated by the leaf-node. 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. This shouldn't be needed. Fundamentally, there is no difference between the root field and any other. Where we're using it below - does this not recreate the same issue we're trying to fix? E.g., if the root field allows 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. No, excuse me, I was reading the code backwards. I think the change creates the problem for non-root fields? Shouldn't we just always call 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. The problem I was seeing was that unless I have this condition (just
This is because the SimpleTreeSchema.allowedTypes gives the 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. What is the root schema in this example? 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. The one that is in the This is also why I have the condition in the 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. Oh, I see. In that case, an easier fix might just be to take in the 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. But I would recommend taking in a list of types, rather than the concatenated string. |
||
): Promise<VisualSharedTreeNode> { | ||
const schemaFactory = new SchemaFactory(undefined); | ||
|
||
return Tree.is(tree, [ | ||
schemaFactory.boolean, | ||
schemaFactory.null, | ||
schemaFactory.number, | ||
schemaFactory.handle, | ||
schemaFactory.string, | ||
]) | ||
? { | ||
schema: { | ||
schemaName: Tree.schema(tree).identifier, | ||
allowedTypes: | ||
isRootField === true | ||
? concatenateTypes(treeSchema.allowedTypes) | ||
: Tree.schema(tree).identifier, | ||
}, | ||
value: await visualizeChildData(tree), | ||
kind: VisualSharedTreeNodeKind.LeafNode, | ||
} | ||
: visualizeInternalNodeBySchema(tree, treeSchema, visualizeChildData); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1730,6 +1730,66 @@ describe("DefaultVisualizers unit tests", () => { | |
expect(result).to.deep.equal(expected); | ||
}); | ||
|
||
it.only("SharedTree: Renders multiple allowed types in SharedTree's root field", async () => { | ||
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. Will be removing |
||
const factory = SharedTree.getFactory(); | ||
const builder = new SchemaFactory("shared-tree-test"); | ||
|
||
const sharedTree = factory.create( | ||
new MockFluidDataStoreRuntime({ idCompressor: createIdCompressor() }), | ||
"test", | ||
); | ||
|
||
const view = sharedTree.viewWith( | ||
new TreeViewConfiguration({ schema: [builder.string, builder.number] }), | ||
); | ||
view.initialize(23); | ||
|
||
const result = await visualizeSharedTree( | ||
sharedTree as unknown as ISharedObject, | ||
visualizeChildData, | ||
); | ||
|
||
const expected = { | ||
children: { | ||
root: { | ||
value: 23, | ||
nodeKind: "ValueNode", | ||
tooltipContents: { | ||
schema: { | ||
nodeKind: "TreeNode", | ||
children: { | ||
name: { | ||
nodeKind: "ValueNode", | ||
value: "com.fluidframework.leaf.number", | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
nodeKind: "FluidTreeNode", | ||
tooltipContents: { | ||
schema: { | ||
nodeKind: "TreeNode", | ||
children: { | ||
name: { | ||
nodeKind: "ValueNode", | ||
value: "root", | ||
}, | ||
allowedTypes: { | ||
value: "com.fluidframework.leaf.string | com.fluidframework.leaf.number", | ||
nodeKind: "ValueNode", | ||
}, | ||
}, | ||
}, | ||
}, | ||
fluidObjectId: "test", | ||
typeMetadata: "SharedTree", | ||
}; | ||
|
||
expect(result).to.deep.equal(expected); | ||
}); | ||
|
||
it("Unknown SharedObject", async () => { | ||
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions | ||
const unknownObject = { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -189,29 +189,46 @@ export class AppData extends DataObject { | |
childData: builder.optional(LeafSchema), | ||
}) {} | ||
|
||
class RootNodeSchema extends builder.object("root-item", { | ||
class RootNodeTwoItemTwo extends builder.object("root-node-two-item-two", { | ||
childrenOne: builder.array(ChildSchema), | ||
childrenTwo: builder.number, | ||
}) {} | ||
|
||
const config = new TreeViewConfiguration({ schema: RootNodeSchema }); | ||
class RootNodeTwoItem extends builder.object("root-node-item", { | ||
childrenOne: builder.number, | ||
childrenTwo: RootNodeTwoItemTwo, | ||
}) {} | ||
|
||
class RootNodeOne extends builder.object("root-node-one", { | ||
leafField: [builder.boolean, builder.handle, builder.string], | ||
}) {} | ||
|
||
class RootNodeTwo extends builder.object("root-node-two", { | ||
childField: builder.optional(RootNodeTwoItem), | ||
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.
|
||
}) {} | ||
|
||
const config = new TreeViewConfiguration({ | ||
schema: [RootNodeOne, RootNodeTwo, builder.string, builder.number], | ||
}); | ||
const view = sharedTree.viewWith(config); | ||
view.initialize({ | ||
childrenOne: [ | ||
{ | ||
childField: "Hello world!", | ||
childData: { | ||
leafField: "Hello world again!", | ||
}, | ||
childField: { | ||
childrenOne: 42, | ||
childrenTwo: { | ||
childrenOne: [ | ||
{ | ||
childField: false, | ||
childData: { | ||
leafField: "leaf data", | ||
}, | ||
}, | ||
{ | ||
childField: true, | ||
}, | ||
], | ||
childrenTwo: 123, | ||
}, | ||
{ | ||
childField: true, | ||
childData: { | ||
leafField: false, | ||
}, | ||
}, | ||
], | ||
childrenTwo: 32, | ||
}, | ||
}); | ||
} | ||
} |
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.
Initializing the view with
view.initialize({})
will render: