refactor(tree): use field cursors in low-level editing API (#19009)

packages/dds/tree/api-report/tree.api.md

@@ -1281,7 +1281,7 @@ export interface Optional extends FlexFieldKind<"Optional", Multiplicity.Optiona
 
 // @internal (undocumented)
 export interface OptionalFieldEditBuilder {
-    set(newContent: ITreeCursor | undefined, wasEmpty: boolean): void;
+    set(newContent: ITreeCursorSynchronous | undefined, wasEmpty: boolean): void;
 }
 
 // @internal
@@ -1530,7 +1530,7 @@ export interface Sequence extends FlexFieldKind<"Sequence", Multiplicity.Sequenc
 // @internal (undocumented)
 export interface SequenceFieldEditBuilder {
     delete(index: number, count: number): void;
-    insert(index: number, newContent: ITreeCursor | readonly ITreeCursor[]): void;
+    insert(index: number, newContent: ITreeCursorSynchronous): void;
     move(sourceIndex: number, count: number, destIndex: number): void;
 }
 
@@ -1951,7 +1951,7 @@ export type Value = undefined | TreeValue;
 
 // @internal (undocumented)
 export interface ValueFieldEditBuilder {
-    set(newContent: ITreeCursor): void;
+    set(newContent: ITreeCursorSynchronous): void;
 }
 
 // @internal

packages/dds/tree/src/feature-libraries/contextuallyTyped.ts

@@ -13,7 +13,8 @@ import {
 	ITreeCursorSynchronous,
 	isCursor,
 	TreeValue,
-} from "../core";
+	CursorLocationType,
+} from "../core/index.js";
 // TODO:
 // This module currently is assuming use of default-field-kinds.
 // The field kinds should instead come from a view schema registry thats provided somewhere.
@@ -31,8 +32,8 @@ import {
 	TreeFieldSchema,
 	TreeNodeSchema,
 	allowedTypesSchemaSet,
-} from "./typed-schema";
-import { cursorForMapTreeNode } from "./mapTreeCursor";
+} from "./typed-schema/index.js";
+import { cursorForMapTreeField, cursorForMapTreeNode, mapTreeFromCursor } from "./mapTreeCursor.js";
 import {
 	AllowedTypesToFlexInsertableTree,
 	InsertableFlexField,
@@ -368,17 +369,17 @@ export function cursorForTypedData<T extends AllowedTypes>(
 
 /**
  * Construct a tree from ContextuallyTypedNodeData.
+ * Returns a cursor in Field mode.
  *
  * TODO: this should probably be refactored into a `try` function which either returns a Cursor or a SchemaError with a path to the error.
- * TODO: migrate APIs which take arrays of cursors to take cursors in fields mode.
  */
 export function cursorsFromContextualData(
 	context: TreeDataContext,
 	field: TreeFieldSchema,
 	data: ContextuallyTypedNodeData | undefined,
-): ITreeCursorSynchronous[] {
+): ITreeCursorSynchronous {
 	const mapTrees = applyFieldTypesFromContext(context, field, data);
-	return mapTrees.map(cursorForMapTreeNode);
+	return cursorForMapTreeField(mapTrees);
 }
 
 /**
@@ -389,7 +390,7 @@ export function cursorsForTypedFieldData<T extends TreeFieldSchema>(
 	context: TreeDataContext,
 	schema: T,
 	data: InsertableFlexField<T>,
-): ITreeCursorSynchronous[] {
+): ITreeCursorSynchronous {
 	return cursorsFromContextualData(context, schema, data as ContextuallyTypedNodeData);
 }
 
@@ -556,27 +557,29 @@ export type NewFieldContent =
 	| ContextuallyTypedFieldData;
 
 /**
- * Convert NewFieldContent into ITreeCursor array.
+ * Convert NewFieldContent into ITreeCursorSynchronous.
+ * The returned cursor will be in Field mode.
  */
 export function normalizeNewFieldContent(
 	context: TreeDataContext,
 	schema: TreeFieldSchema,
 	content: NewFieldContent,
-): readonly ITreeCursorSynchronous[] {
+): ITreeCursorSynchronous {
 	if (areCursors(content)) {
-		if (getFieldKind(schema).multiplicity === Multiplicity.Sequence) {
-			assert(isReadonlyArray(content), 0x6b7 /* sequence fields require array content */);
+		if (isReadonlyArray(content)) {
+			assert(
+				getFieldKind(schema).multiplicity === Multiplicity.Sequence || content.length === 1,
+				0x6b8 /* non-sequence fields can not be provided content that is multiple cursors */,
+			);
+			// TODO: is there a better way to get a field cursor from an array of node cursors?
+			const mapTrees = content.map((c) => mapTreeFromCursor(c));
+			return cursorForMapTreeField(mapTrees);
+		}
+		if (content.mode === CursorLocationType.Fields) {
 			return content;
-		} else {
-			if (isReadonlyArray(content)) {
-				assert(
-					content.length === 1,
-					0x6b8 /* non-sequence fields can not be provided content that is multiple cursors */,
-				);
-				return content;
-			}
-			return [content];
 		}
+		// TODO: is there a better way to get a field cursor from a node cursor?
+		return cursorForMapTreeField([mapTreeFromCursor(content)]);
 	}
 
 	return cursorsFromContextualData(context, schema, content);

packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts

@@ -11,7 +11,6 @@ import {
 	ChangeFamily,
 	ChangeRebaser,
 	UpPath,
-	ITreeCursor,
 	ChangeFamilyEditor,
 	FieldUpPath,
 	compareFieldUpPaths,
@@ -21,8 +20,10 @@ import {
 	ChangesetLocalId,
 	DeltaDetachedNodeId,
 	ChangeEncodingContext,
-} from "../../core";
-import { brand, isReadonlyArray } from "../../util";
+	CursorLocationType,
+	ITreeCursorSynchronous,
+} from "../../core/index.js";
+import { brand } from "../../util/index.js";
 import {
 	ModularChangeFamily,
 	ModularEditBuilder,
@@ -174,10 +175,10 @@ export class DefaultEditBuilder implements ChangeFamilyEditor, IDefaultEditBuild
 
 	public valueField(field: FieldUpPath): ValueFieldEditBuilder {
 		return {
-			set: (newContent: ITreeCursor): void => {
+			set: (newContent: ITreeCursorSynchronous): void => {
 				const fillId = this.modularBuilder.generateId();
 
-				const build = this.modularBuilder.buildTrees(fillId, [newContent]);
+				const build = this.modularBuilder.buildTrees(fillId, newContent);
 				const change: FieldChangeset = brand(
 					valueFieldKind.changeHandler.editor.set({
 						fill: fillId,
@@ -198,14 +199,14 @@ export class DefaultEditBuilder implements ChangeFamilyEditor, IDefaultEditBuild
 
 	public optionalField(field: FieldUpPath): OptionalFieldEditBuilder {
 		return {
-			set: (newContent: ITreeCursor | undefined, wasEmpty: boolean): void => {
+			set: (newContent: ITreeCursorSynchronous | undefined, wasEmpty: boolean): void => {
 				const detachId = this.modularBuilder.generateId();
 				let fillId: ChangesetLocalId | undefined;
 				const edits: EditDescription[] = [];
 				let optionalChange: OptionalChangeset;
 				if (newContent !== undefined) {
 					fillId = this.modularBuilder.generateId();
-					const build = this.modularBuilder.buildTrees(fillId, [newContent]);
+					const build = this.modularBuilder.buildTrees(fillId, newContent);
 					edits.push(build);
 
 					optionalChange = optional.changeHandler.editor.set(wasEmpty, {
@@ -313,9 +314,9 @@ export class DefaultEditBuilder implements ChangeFamilyEditor, IDefaultEditBuild
 
 	public sequenceField(field: FieldUpPath): SequenceFieldEditBuilder {
 		return {
-			insert: (index: number, newContent: ITreeCursor | readonly ITreeCursor[]): void => {
-				const content = isReadonlyArray(newContent) ? newContent : [newContent];
-				const length = content.length;
+			insert: (index: number, content: ITreeCursorSynchronous): void => {
+				const length =
+					content.mode === CursorLocationType.Fields ? content.getFieldLength() : 1;
 				if (length === 0) {
 					return;
 				}
@@ -368,9 +369,10 @@ export class DefaultEditBuilder implements ChangeFamilyEditor, IDefaultEditBuild
 export interface ValueFieldEditBuilder {
 	/**
 	 * Issues a change which replaces the current newContent of the field with `newContent`.
-	 * @param newContent - the new content for the field. Must be in Nodes mode.
+	 * @param newContent - the new content for the field.
+	 * The cursor can be in either Field or Node mode and must represent exactly one node.
 	 */
-	set(newContent: ITreeCursor): void;
+	set(newContent: ITreeCursorSynchronous): void;
 }
 
 /**
@@ -379,10 +381,11 @@ export interface ValueFieldEditBuilder {
 export interface OptionalFieldEditBuilder {
 	/**
 	 * Issues a change which replaces the current newContent of the field with `newContent`
-	 * @param newContent - the new content for the field. Must be in Nodes mode.
+	 * @param newContent - the new content for the field.
+	 * If provided, the cursor can be in either Field or Node mode and must represent exactly one node.
 	 * @param wasEmpty - whether the field is empty when creating this change
 	 */
-	set(newContent: ITreeCursor | undefined, wasEmpty: boolean): void;
+	set(newContent: ITreeCursorSynchronous | undefined, wasEmpty: boolean): void;
 }
 
 /**
@@ -392,9 +395,9 @@ export interface SequenceFieldEditBuilder {
 	/**
 	 * Issues a change which inserts the `newContent` at the given `index`.
 	 * @param index - the index at which to insert the `newContent`.
-	 * @param newContent - the new content to be inserted in the field. Cursors must be in Nodes mode.
+	 * @param newContent - the new content to be inserted in the field. Cursor can be in either Field or Node mode.
 	 */
-	insert(index: number, newContent: ITreeCursor | readonly ITreeCursor[]): void;
+	insert(index: number, newContent: ITreeCursorSynchronous): void;
 
 	/**
 	 * Issues a change which deletes `count` elements starting at the given `index`.

packages/dds/tree/src/feature-libraries/flex-tree/lazyField.ts

@@ -17,22 +17,27 @@ import {
 	iterateCursorField,
 	isCursor,
 	ITreeCursorSynchronous,
-	mapCursorField,
-} from "../../core";
-import { FieldKind } from "../modular-schema";
+} from "../../core/index.js";
+import { FieldKind } from "../modular-schema/index.js";
 // TODO: stop depending on contextuallyTyped
-import { cursorFromContextualData } from "../contextuallyTyped";
+import { applyTypesFromContext, cursorFromContextualData } from "../contextuallyTyped.js";
 import {
 	FieldKinds,
 	OptionalFieldEditBuilder,
 	SequenceFieldEditBuilder,
 	ValueFieldEditBuilder,
-} from "../default-schema";
-import { assertValidIndex, assertValidRangeIndices, brand, disposeSymbol, fail } from "../../util";
-import { AllowedTypes, TreeFieldSchema } from "../typed-schema";
-import { LocalNodeKey, StableNodeKey, nodeKeyTreeIdentifier } from "../node-key";
-import { mapTreeFromCursor, cursorForMapTreeNode } from "../mapTreeCursor";
-import { Context } from "./context";
+} from "../default-schema/index.js";
+import {
+	assertValidIndex,
+	assertValidRangeIndices,
+	brand,
+	disposeSymbol,
+	fail,
+} from "../../util/index.js";
+import { AllowedTypes, TreeFieldSchema } from "../typed-schema/index.js";
+import { LocalNodeKey, StableNodeKey, nodeKeyTreeIdentifier } from "../node-key/index.js";
+import { cursorForMapTreeField } from "../mapTreeCursor.js";
+import { Context } from "./context.js";
 import {
 	FlexibleNodeContent,
 	FlexTreeOptionalField,
@@ -301,11 +306,14 @@ export class LazySequence<TTypes extends AllowedTypes>
 
 	public insertAt(index: number, value: FlexibleNodeSubSequence<TTypes>): void {
 		assertValidIndex(index, this, true);
-		const content: ITreeCursorSynchronous[] = isCursor(value)
+		const content: ITreeCursorSynchronous = isCursor(value)
 			? prepareFieldCursorForInsert(value)
-			: Array.from(value, (item) =>
-					cursorFromContextualData(this.context, this.schema.allowedTypeSet, item),
+			: cursorForMapTreeField(
+					Array.from(value, (item) =>
+						applyTypesFromContext(this.context, this.schema.allowedTypeSet, item),
+					),
 			  );
+
 		const fieldEditor = this.sequenceEditor();
 		fieldEditor.insert(index, content);
 	}
@@ -578,13 +586,11 @@ const kindToClass: ReadonlyMap<FieldKind, Builder> = new Map(builderList);
 /**
  * Prepare a fields cursor (holding a sequence of nodes) for inserting.
  */
-function prepareFieldCursorForInsert(cursor: ITreeCursorSynchronous): ITreeCursorSynchronous[] {
+function prepareFieldCursorForInsert(cursor: ITreeCursorSynchronous): ITreeCursorSynchronous {
 	// TODO: optionally validate content against schema.
 
-	// Convert from the desired API (single field cursor) to the currently required API (array of node cursors).
-	// This is inefficient, and particularly bad if the data was efficiently chunked using uniform chunks.
-	// TODO: update editing APIs to take in field cursors not arrays of node cursors, then remove this copying conversion.
-	return mapCursorField(cursor, () => cursorForMapTreeNode(mapTreeFromCursor(cursor)));
+	assert(cursor.mode === CursorLocationType.Fields, "should be in fields mode");
+	return cursor;
 }
 
 /**

packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts

@@ -26,7 +26,6 @@ import {
 	revisionMetadataSourceFromInfo,
 	ChangeAtomIdMap,
 	makeDetachedNodeId,
-	ITreeCursor,
 	emptyDelta,
 	DeltaFieldMap,
 	DeltaFieldChanges,
@@ -37,7 +36,9 @@ import {
 	ChangeEncodingContext,
 	RevisionTagCodec,
 	mapCursorField,
-} from "../../core";
+	ITreeCursorSynchronous,
+	CursorLocationType,
+} from "../../core/index.js";
 import {
 	brand,
 	deleteFromNestedMap,
@@ -48,7 +49,6 @@ import {
 	IdAllocator,
 	idAllocatorFromMaxId,
 	idAllocatorFromState,
-	isReadonlyArray,
 	Mutable,
 	tryGetFromNestedMap,
 } from "../../util";
@@ -58,8 +58,9 @@ import {
 	chunkFieldSingle,
 	defaultChunkPolicy,
 	FieldBatchCodec,
-} from "../chunked-forest";
-import { cursorForMapTreeField, cursorForMapTreeNode, mapTreeFromCursor } from "../mapTreeCursor";
+	chunkTree,
+} from "../chunked-forest/index.js";
+import { cursorForMapTreeNode, mapTreeFromCursor } from "../mapTreeCursor.js";
 import {
 	CrossFieldManager,
 	CrossFieldMap,
@@ -1272,22 +1273,25 @@ export class ModularEditBuilder extends EditBuilder<ModularChangeset> {
 		}
 	}
 
+	/**
+	 * @param firstId - The ID to associate with the first node
+	 * @param content - The node(s) to build. Can be in either Field or Node mode.
+	 * @returns A description of the edit that can be passed to `submitChanges`.
+	 */
 	public buildTrees(
 		firstId: ChangesetLocalId,
-		newContent: ITreeCursor | readonly ITreeCursor[],
+		content: ITreeCursorSynchronous,
 	): GlobalEditDescription {
-		const content = isReadonlyArray(newContent) ? newContent : [newContent];
-		const length = content.length;
-		if (length === 0) {
+		if (content.mode === CursorLocationType.Fields && content.getFieldLength() === 0) {
 			return { type: "global" };
 		}
 		const builds: ChangeAtomIdMap<TreeChunk> = new Map();
 		const innerMap = new Map();
 		builds.set(undefined, innerMap);
-
-		const mapTrees = content.map((c) => mapTreeFromCursor(c));
-		const fieldCursor = cursorForMapTreeField(mapTrees);
-		const chunk = chunkFieldSingle(fieldCursor, defaultChunkPolicy);
+		const chunk =
+			content.mode === CursorLocationType.Fields
+				? chunkFieldSingle(content, defaultChunkPolicy)
+				: chunkTree(content, defaultChunkPolicy);
 		innerMap.set(firstId, chunk);
 
 		return {

packages/dds/tree/src/shared-tree/sharedTree.ts

@@ -344,10 +344,10 @@ export class SharedTree
 					case FieldKinds.optional.identifier: {
 						const fieldEditor = this.editor.optionalField(field);
 						assert(
-							content.length <= 1,
+							content.getFieldLength() <= 1,
 							0x7f4 /* optional field content should normalize at most one item */,
 						);
-						fieldEditor.set(content.length === 0 ? undefined : content[0], true);
+						fieldEditor.set(content.getFieldLength() === 0 ? undefined : content, true);
 						break;
 					}
 					case FieldKinds.sequence.identifier: {

packages/dds/tree/src/test/shared-tree/editing.spec.ts

@@ -1694,7 +1694,7 @@ describe("Editing", () => {
 				parent: rootNode,
 				field: brand("foo"),
 			});
-			field.insert(1, [cursorForJsonableTreeNode({ type: leaf.string.name, value: "C" })]);
+			field.insert(1, cursorForJsonableTreeNode({ type: leaf.string.name, value: "C" }));
 			assert.equal(valueAfterInsert, "C");
 			unsubscribePathVisitor();
 		});