Skip to content

Commit

Permalink
Hydrate nodes in the initial tree passed to SharedTree's view initial…
Browse files Browse the repository at this point in the history
…ization (#21565)

## Description

Previously, nodes in the initial tree were not hydrated (as they are
when inserting content in general). This meant that users holding on to
references to the nodes passed to initialization would not find them to
be the same as the nodes that are read out of the tree after
initialization. This PR updates the behavior to be the same as when
inserting nodes - the inserted nodes can be used after the insertion.
  • Loading branch information
noencke authored Jun 20, 2024
1 parent c07649e commit c6b1941
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 23 deletions.
23 changes: 18 additions & 5 deletions packages/dds/tree/src/shared-tree/schematizingTreeView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
ViewSchema,
defaultSchemaPolicy,
ContextSlot,
cursorForMapTreeNode,
} from "../feature-libraries/index.js";
import {
type FieldSchema,
Expand All @@ -35,8 +36,9 @@ import {
setField,
normalizeFieldSchema,
type InsertableContent,
cursorFromUnhydratedRoot,
type TreeViewConfiguration,
mapTreeFromNodeData,
prepareContentForHydration,
} from "../simple-tree/index.js";
import { disposeSymbol } from "../util/index.js";

Expand Down Expand Up @@ -127,12 +129,23 @@ export class SchematizingSimpleTreeView<in out TRootSchema extends ImplicitField
}

this.runSchemaEdit(() => {
const mapTree = mapTreeFromNodeData(
content as InsertableContent,
this.rootFieldSchema.allowedTypes,
this.nodeKeyManager,
{
schema: this.checkout.storedSchema,
policy: {
...defaultSchemaPolicy,
validateSchema: this.config.enableSchemaValidation,
},
},
);

prepareContentForHydration(mapTree, this.checkout.forest);
initialize(this.checkout, {
schema: this.flexConfig.schema,
initialTree:
content === undefined
? undefined
: cursorFromUnhydratedRoot(this.config.schema, content, this.nodeKeyManager),
initialTree: mapTree === undefined ? undefined : cursorForMapTreeNode(mapTree),
});
});
}
Expand Down
7 changes: 6 additions & 1 deletion packages/dds/tree/src/simple-tree/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,11 @@ export type {
NodeFromSchemaUnsafe,
} from "./typesUnsafe.js";
export type { ValidateRecursiveSchema } from "./schemaFactoryRecursive.js";
export { getProxyForField, type InsertableContent } from "./proxies.js";
export {
getProxyForField,
type InsertableContent,
prepareContentForHydration,
} from "./proxies.js";

export {
adaptEnum,
Expand Down Expand Up @@ -93,3 +97,4 @@ export {
setField,
} from "./objectNode.js";
export type { TreeMapNode } from "./mapNode.js";
export { mapTreeFromNodeData } from "./toMapTree.js";
3 changes: 1 addition & 2 deletions packages/dds/tree/src/test/shared-tree/sharedTree.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,7 @@ describe("SharedTree", () => {
assert.equal(schematized.flexTree.content, undefined);
});

// TODO: ensure unhydrated initialTree input is correctly hydrated.
it.skip("unhydrated tree input", () => {
it("unhydrated tree input", () => {
const tree = DebugSharedTree.create(new MockSharedTreeRuntime());
const sb = new SchemaFactory("test-factory");
class Foo extends sb.object("Foo", {}) {}
Expand Down
27 changes: 18 additions & 9 deletions packages/dds/tree/src/test/simple-tree/schemaFactory.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,8 +381,7 @@ describe("schemaFactory", () => {
assert.notDeepEqual(p1, p2);
});

// Walking unhydrated nodes is currently not supported
it.skip("unhydrated", () => {
it("unhydrated", () => {
assert.deepEqual(new Point({ x: 1, y: 2 }), new Point({ x: 1, y: 2 }));
assert.notDeepEqual(new Point({ x: 1, y: 2 }), new Point({ x: 1, y: 3 }));
assert.notDeepEqual(new Point({ x: 1, y: 2 }), { x: 1, y: 2 });
Expand Down Expand Up @@ -597,7 +596,7 @@ describe("schemaFactory", () => {
ComboChildMap,
]) {}
class ComboRoot extends comboSchemaFactory.object("comboRoot", {
root: comboSchemaFactory.optional([ComboParentObject, ComboParentList, ComboParentMap]),
root: [ComboParentObject, ComboParentList, ComboParentMap],
}) {}

type ComboParent = ComboParentObject | ComboParentList | ComboParentMap;
Expand Down Expand Up @@ -730,9 +729,19 @@ describe("schemaFactory", () => {
new MockFluidDataStoreRuntime({ idCompressor: createIdCompressor() }),
"tree",
);

// Check that nodes in the initial tree are hydrated
const view = tree.viewWith(config);
view.initialize({ root: undefined });
const { parent, nodes } = createComboTree({
const { parent: initialParent, nodes: initialNodes } = createComboTree({
parentType,
childType,
});

view.initialize({ root: initialParent });
validate(view, initialNodes);

// Check that nodes inserted later are hydrated
const { parent: insertedParent, nodes: insertedNodes } = createComboTree({
parentType,
childType,
});
Expand All @@ -741,10 +750,10 @@ describe("schemaFactory", () => {
// Note: as of 2024-03-28, we can't easily test 'treeChanged' because it can fire at a time where the changes
// to the tree are not visible in the listener. 'nodeChanged' only fires once we confirmed that a
// relevant change was actually applied to the tree so the side effects this test validates already happened.
Tree.on(view.root, "nodeChanged", () => validate(view, nodes));
view.events.on("rootChanged", () => validate(view, nodes));
view.root.root = parent;
validate(view, nodes);
Tree.on(view.root, "nodeChanged", () => validate(view, insertedNodes));
view.events.on("rootChanged", () => validate(view, insertedNodes));
view.root.root = insertedParent;
validate(view, insertedNodes);
}

for (const parentType of objectTypes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,12 +300,13 @@ for (const testOpts of testMatrix) {
new TreeViewConfiguration({ schema: Doll, enableSchemaValidation: true }),
);

view.initialize(new Doll({ nested: new Doll({ nested: new Doll({}) }) }));
const depth0 = view.root;
const depth1 = depth0.nested;
assert(depth1 !== undefined);
const depth2 = depth1.nested;
assert(depth2 !== undefined);
// These nodes in the initial tree are unhydrated...
const depth1 = new Doll({ nested: new Doll({}) });
const depth0 = new Doll({ nested: depth1 });
view.initialize(depth0);
// ...and confirmed to be the same nodes we get when we read the tree after initialization
assert.equal(view.root, depth0);
assert.equal(view.root.nested, depth1);
// Record a list of the node events fired
const eventLog: string[] = [];
Tree.on(depth0, "nodeChanged", () => {
Expand Down

0 comments on commit c6b1941

Please sign in to comment.