Skip to content

Cleanup unhydrated tree and tree import code more generally #24740

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

Merged
merged 10 commits into from
May 31, 2025
29 changes: 22 additions & 7 deletions packages/dds/tree/src/core/tree/mapTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
* Licensed under the MIT License.
*/

import { assert } from "@fluidframework/core-utils/internal";

import type { MinimalMapTreeNodeView } from "../../feature-libraries/index.js";
import type { FieldKey } from "../schema-stored/index.js";

import type { NodeData } from "./types.js";
Expand Down Expand Up @@ -36,17 +39,32 @@ export interface ExclusiveMapTree extends NodeData, MapTree {
* @privateRemarks This is implemented iteratively (rather than recursively, which is much simpler)
* to avoid the possibility of a stack overflow for very deep trees.
*/
export function deepCopyMapTree(mapTree: MapTree): ExclusiveMapTree {
type Next = [fields: ExclusiveMapTree["fields"], sourceFields: MapTree["fields"]];
export function deepCopyMapTree(mapTree: MinimalMapTreeNodeView): ExclusiveMapTree {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can now copy from any MinimalMapTreeNodeView: an interface UnhydratedFlexTree nodes start implementing in #24739

type Next = [
fields: ExclusiveMapTree["fields"],
sourceFields: MinimalMapTreeNodeView["fields"],
];
function create(
child: MinimalMapTreeNodeView,
fields: ExclusiveMapTree["fields"],
): ExclusiveMapTree {
return {
type: child.type,
...(child.value !== undefined ? { value: child.value } : {}),
fields,
};
}

const rootFields: ExclusiveMapTree["fields"] = new Map();
const nexts: Next[] = [[rootFields, mapTree.fields]];
for (let next = nexts.pop(); next !== undefined; next = nexts.pop()) {
const [fields, sourceFields] = next;
for (const [key, field] of sourceFields) {
assert(field.length > 0, "invalid map tree: empty field");
if (field.length > 0) {
const newField: ExclusiveMapTree[] = [];
for (const child of field) {
const childClone: ExclusiveMapTree = { ...child, fields: new Map() };
const childClone = create(child, new Map());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the use of spread here and below broke when using implementations of where type or value as a getter (not enumerable) or there were other enumerable properties which should not be copied.

spread was used to avoid including the value property when not present, but that has been reimplemented in create above.

newField.push(childClone);
nexts.push([childClone.fields, child.fields]);
}
Expand All @@ -55,8 +73,5 @@ export function deepCopyMapTree(mapTree: MapTree): ExclusiveMapTree {
}
}

return {
...mapTree,
fields: rootFields,
};
return create(mapTree, rootFields);
}
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ export interface OptionalFieldEditBuilder<TContent> {

/**
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any docs we can add to this? Alternatively, can we remove the blank comment block?

export interface SequenceFieldEditBuilder<TContent> {
export interface SequenceFieldEditBuilder<TContent, TRemoved = void> {
/**
* Issues a change which inserts the `newContent` at the given `index`.
* @param index - the index at which to insert the `newContent`.
Expand All @@ -448,7 +448,7 @@ export interface SequenceFieldEditBuilder<TContent> {
* @param index - The index of the first removed element.
* @param count - The number of elements to remove.
*/
remove(index: number, count: number): void;
remove(index: number, count: number): TRemoved;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unhydrated flex tree implements returning the results from remove. This allows that to be expressed more clearly.

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { unreachableCase, fail } from "@fluidframework/core-utils/internal";
import { UsageError } from "@fluidframework/telemetry-utils/internal";

import {
type MapTree,
type TreeFieldStoredSchema,
LeafNodeStoredSchema,
ObjectNodeStoredSchema,
Expand All @@ -16,6 +15,7 @@ import {
type SchemaAndPolicy,
} from "../../core/index.js";
import { allowsValue } from "../valueUtilities.js";
import type { MapTreeFieldViewGeneric, MinimalMapTreeNodeView } from "../mapTreeCursor.js";

export enum SchemaValidationError {
Field_KindNotInSchemaPolicy,
Expand Down Expand Up @@ -45,7 +45,7 @@ export function inSchemaOrThrow(maybeError: SchemaValidationError | undefined):
* Deeply checks that the provided node complies with the schema based on its identifier.
*/
export function isNodeInSchema(
node: MapTree,
node: MinimalMapTreeNodeView,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More general type, so it can apply to unhydrated flex tree nodes in #24739

schemaAndPolicy: SchemaAndPolicy,
): SchemaValidationError | undefined {
// Validate the schema declared by the node exists
Expand Down Expand Up @@ -86,7 +86,7 @@ export function isNodeInSchema(
return SchemaValidationError.ObjectNode_FieldNotInSchema;
}
} else if (schema instanceof MapNodeStoredSchema) {
for (const field of node.fields.values()) {
for (const [_key, field] of node.fields) {
const fieldInSchemaResult = isFieldInSchema(field, schema.mapFields, schemaAndPolicy);
if (fieldInSchemaResult !== undefined) {
return fieldInSchemaResult;
Expand All @@ -104,7 +104,7 @@ export function isNodeInSchema(
* Deeply checks that the nodes comply with the field schema and included schema.
*/
export function isFieldInSchema(
childNodes: readonly MapTree[],
childNodes: MapTreeFieldViewGeneric<MinimalMapTreeNodeView>,
schema: TreeFieldStoredSchema,
schemaAndPolicy: SchemaAndPolicy,
): SchemaValidationError | undefined {
Expand Down
23 changes: 15 additions & 8 deletions packages/dds/tree/src/feature-libraries/flex-tree/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,23 +53,30 @@ export interface FlexTreeContext {
isDisposed(): boolean;
}

/**
* Subset of a hydrated context which can be used in more cases (like before the root and events are set up).
*/
export interface FlexTreeHydratedContextMinimal {
readonly nodeKeyManager: NodeIdentifierManager;

/**
* The checkout object associated with this context.
*/
readonly checkout: ITreeCheckout;
}

/**
* A common context of a "forest" of FlexTrees.
* It handles group operations like transforming cursors into anchors for edits.
*/
export interface FlexTreeHydratedContext extends FlexTreeContext {
export interface FlexTreeHydratedContext
extends FlexTreeContext,
FlexTreeHydratedContextMinimal {
readonly events: Listenable<ForestEvents>;
/**
* Gets the root field of the tree.
*/
get root(): FlexTreeField;

readonly nodeKeyManager: NodeIdentifierManager;

/**
* The checkout object associated with this context.
*/
readonly checkout: ITreeCheckout;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ export interface FlexTreeField extends FlexTreeEntity {
/**
* Typed tree for inserting as the content of a field.
*/
export type FlexibleFieldContent = ExclusiveMapTree[];
export type FlexibleFieldContent = readonly FlexibleNodeContent[];

/**
* Tree for inserting as a node.
Expand Down
3 changes: 3 additions & 0 deletions packages/dds/tree/src/feature-libraries/flex-tree/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ export {
FlexTreeEntityKind,
isFlexTreeNode,
flexTreeSlot,
type FlexibleNodeContent,
type FlexibleFieldContent,
} from "./flexTreeTypes.js";

export {
Expand All @@ -32,6 +34,7 @@ export {
type FlexTreeHydratedContext,
Context,
ContextSlot,
type FlexTreeHydratedContextMinimal,
} from "./context.js";

export { type FlexTreeNodeEvents } from "./treeEvents.js";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ import {
type FlexTreeSequenceField,
type FlexTreeTypedField,
type FlexTreeUnknownUnboxed,
type FlexibleFieldContent,
type FlexibleNodeContent,
TreeStatus,
flexTreeMarker,
flexTreeSlot,
Expand Down Expand Up @@ -263,7 +265,7 @@ export class LazySequence extends LazyField implements FlexTreeSequenceField {
return this.map((x) => x);
}

public editor: SequenceFieldEditBuilder<ExclusiveMapTree[]> = {
public editor: SequenceFieldEditBuilder<FlexibleFieldContent> = {
insert: (index, newContent) => {
this.sequenceEditor().insert(index, cursorForMapTreeField(newContent));
},
Expand All @@ -279,7 +281,7 @@ export class LazySequence extends LazyField implements FlexTreeSequenceField {
}

export class LazyValueField extends LazyField implements FlexTreeRequiredField {
public editor: ValueFieldEditBuilder<ExclusiveMapTree> = {
public editor: ValueFieldEditBuilder<FlexibleNodeContent> = {
set: (newContent) => {
this.valueFieldEditor().set(cursorForMapTreeField([newContent]));
},
Expand Down
6 changes: 6 additions & 0 deletions packages/dds/tree/src/feature-libraries/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ export {
cursorForMapTreeNode,
mapTreeFromCursor,
mapTreeFieldFromCursor,
type MinimalMapTreeNodeView,
mapTreeFieldsWithField,
mapTreeWithField,
} from "./mapTreeCursor.js";
export { buildForest } from "./object-forest/index.js";
export {
Expand Down Expand Up @@ -174,6 +177,9 @@ export {
treeStatusFromAnchorCache,
indexForAt,
FlexTreeEntityKind,
type FlexibleNodeContent,
type FlexibleFieldContent,
type FlexTreeHydratedContextMinimal,
} from "./flex-tree/index.js";

export { TreeCompressionStrategy } from "./treeCompressionUtils.js";
Expand Down
94 changes: 79 additions & 15 deletions packages/dds/tree/src/feature-libraries/mapTreeCursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,49 +12,113 @@ import {
type FieldKey,
type ITreeCursor,
type MapTree,
type NodeData,
aboveRootPlaceholder,
detachedFieldAsKey,
mapCursorField,
rootField,
rootFieldKey,
} from "../core/index.js";

import {
type CursorAdapter,
type CursorWithNode,
type Field,
stackTreeFieldCursor,
stackTreeNodeCursor,
} from "./treeCursorUtils.js";
import type { requireAssignableTo } from "../util/index.js";

export interface MapTreeNodeViewGeneric<TNode> extends NodeData {
/**
* The non-empty fields on this node.
*/
readonly fields: Pick<
ReadonlyMap<FieldKey, MapTreeFieldViewGeneric<TNode>>,
typeof Symbol.iterator | "get" | "size" | "keys"
>;
}

export type MapTreeFieldViewGeneric<TNode> = Pick<
readonly TNode[],
typeof Symbol.iterator | "length"
>;

/**
* Like {@link MapTree} but with the minimal properties needed for reading.
*/
export interface MinimalMapTreeNodeView
extends MapTreeNodeViewGeneric<MinimalMapTreeNodeView> {}

{
type _check1 = requireAssignableTo<MapTree, MinimalMapTreeNodeView>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: a quick comment explaining the type checks would probably be good.

type _check2 = requireAssignableTo<MapTree, MapTreeNodeViewGeneric<MapTree>>;
}

/**
* Returns an {@link ITreeCursorSynchronous} in nodes mode for a single {@link MapTree}.
*/
export function cursorForMapTreeNode(root: MapTree): CursorWithNode<MapTree> {
return stackTreeNodeCursor(adapter, root);
export function cursorForMapTreeNode<T extends MapTreeNodeViewGeneric<T>>(
root: T,
): CursorWithNode<T> {
// There doesn't seem to be a clean way to get TypeScript to type check this without casting
// without declaring the adapter inside this generic function and needlessly recreating it on every call.
const adapterTyped = adapter as CursorAdapter<MapTreeNodeViewGeneric<T>> as CursorAdapter<T>;
return stackTreeNodeCursor(adapterTyped, root);
}

export function mapTreeWithField(
children: ExclusiveMapTree[],
key = rootFieldKey,
type = aboveRootPlaceholder,
): MapTree {
return {
type,
fields: mapTreeFieldsWithField(children, key),
};
}

export function mapTreeFieldsWithField<T extends readonly unknown[]>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs?

children: T,
key: FieldKey,
): Map<FieldKey, T> {
return new Map(children.length === 0 ? [] : [[key, children]]);
}

/**
* Returns an {@link ITreeCursorSynchronous} in fields mode for a MapTree field.
*/
export function cursorForMapTreeField(
root: readonly MapTree[],
export function cursorForMapTreeField<T extends MapTreeNodeViewGeneric<T>>(
root: readonly T[],
detachedField: DetachedField = rootField,
): CursorWithNode<MapTree> {
): CursorWithNode<T> {
const key = detachedFieldAsKey(detachedField);
return stackTreeFieldCursor(
adapter,
{
type: aboveRootPlaceholder,
fields: new Map([[key, root]]),
},
detachedField,
);
const adapterTyped = adapter as unknown as CursorAdapter<T>;
const dummyRoot: MapTreeNodeViewGeneric<T> = {
type: aboveRootPlaceholder,
fields: mapTreeFieldsWithField(root, key),
};
return stackTreeFieldCursor(adapterTyped, dummyRoot as T, detachedField);
}

const adapter: CursorAdapter<MapTree> = {
const adapter: CursorAdapter<MinimalMapTreeNodeView> = {
value: (node) => node.value,
type: (node) => node.type,
keysFromNode: (node) => [...node.fields.keys()], // TODO: don't convert this to array here.
getFieldFromNode: (node, key) => node.fields.get(key) ?? [],
getFieldFromNode: (node, key): Field<MinimalMapTreeNodeView> => {
const field = node.fields.get(key) as
| MinimalMapTreeNodeView[]
| Iterable<MinimalMapTreeNodeView>
| undefined;
if (field === undefined) {
return [];
}
if (Array.isArray(field)) {
return field as readonly MinimalMapTreeNodeView[];
}
// TODO: this copy could be avoided by using Array.at instead of indexing in `Field`, but that requires ES2022.
return [...field] as readonly MinimalMapTreeNodeView[];
},
};

/**
Expand Down
Loading
Loading