From ed52a5c20e82ae4ba0653053e45a00e1701a9d05 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Fri, 23 May 2025 15:22:28 -0700 Subject: [PATCH 01/25] Refactor UnhydratedFlexTree and toMapTree --- .../default-schema/schemaChecker.ts | 8 +- .../feature-libraries/flex-tree/context.ts | 20 +- .../flex-tree/flexTreeTypes.ts | 6 +- .../src/feature-libraries/flex-tree/index.ts | 3 + .../feature-libraries/flex-tree/lazyField.ts | 6 +- .../dds/tree/src/feature-libraries/index.ts | 3 + .../src/feature-libraries/mapTreeCursor.ts | 67 +++- .../src/feature-libraries/treeCursorUtils.ts | 28 +- .../dds/tree/src/shared-tree/treeAlpha.ts | 5 +- .../dds/tree/src/simple-tree/api/create.ts | 33 +- .../dds/tree/src/simple-tree/api/index.ts | 5 +- .../tree/src/simple-tree/api/schemaFactory.ts | 27 +- .../tree/src/simple-tree/api/treeNodeApi.ts | 9 +- .../dds/tree/src/simple-tree/core/index.ts | 1 - .../simple-tree/core/unhydratedFlexTree.ts | 371 +++++++----------- packages/dds/tree/src/simple-tree/index.ts | 1 - .../simple-tree/node-kinds/array/arrayNode.ts | 18 +- .../tree/src/simple-tree/node-kinds/index.ts | 1 - .../src/simple-tree/node-kinds/map/mapNode.ts | 12 +- .../simple-tree/node-kinds/object/index.ts | 1 - .../node-kinds/object/objectNode.ts | 50 +-- .../src/simple-tree/prepareForInsertion.ts | 64 +-- .../dds/tree/src/simple-tree/schemaTypes.ts | 15 +- .../dds/tree/src/simple-tree/toMapTree.ts | 267 +++++-------- .../tree/src/simple-tree/toStoredSchema.ts | 5 +- .../shared-tree/schematizingTreeView.spec.ts | 2 +- .../test/simple-tree/treeNodeValid.spec.ts | 4 +- 27 files changed, 442 insertions(+), 590 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts b/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts index c62c5911af04..a5f99923d953 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts @@ -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, @@ -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, @@ -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, schemaAndPolicy: SchemaAndPolicy, ): SchemaValidationError | undefined { // Validate the schema declared by the node exists @@ -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; @@ -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, schema: TreeFieldStoredSchema, schemaAndPolicy: SchemaAndPolicy, ): SchemaValidationError | undefined { diff --git a/packages/dds/tree/src/feature-libraries/flex-tree/context.ts b/packages/dds/tree/src/feature-libraries/flex-tree/context.ts index 9a3cfd81987b..bc80e87fb3e9 100644 --- a/packages/dds/tree/src/feature-libraries/flex-tree/context.ts +++ b/packages/dds/tree/src/feature-libraries/flex-tree/context.ts @@ -53,23 +53,27 @@ export interface FlexTreeContext { isDisposed(): boolean; } +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; /** * Gets the root field of the tree. */ get root(): FlexTreeField; - - readonly nodeKeyManager: NodeIdentifierManager; - - /** - * The checkout object associated with this context. - */ - readonly checkout: ITreeCheckout; } /** diff --git a/packages/dds/tree/src/feature-libraries/flex-tree/flexTreeTypes.ts b/packages/dds/tree/src/feature-libraries/flex-tree/flexTreeTypes.ts index 674772001df3..c8fbfda1f9e7 100644 --- a/packages/dds/tree/src/feature-libraries/flex-tree/flexTreeTypes.ts +++ b/packages/dds/tree/src/feature-libraries/flex-tree/flexTreeTypes.ts @@ -5,7 +5,6 @@ import { type AnchorNode, - type ExclusiveMapTree, type FieldKey, type FieldKindIdentifier, type ITreeCursorSynchronous, @@ -20,6 +19,7 @@ import type { ValueFieldEditBuilder, OptionalFieldEditBuilder, } from "../default-schema/index.js"; +import type { MinimalMapTreeNodeView } from "../mapTreeCursor.js"; import type { FlexFieldKind } from "../modular-schema/index.js"; import type { FlexTreeContext } from "./context.js"; @@ -278,12 +278,12 @@ 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. */ -export type FlexibleNodeContent = ExclusiveMapTree; +export type FlexibleNodeContent = MinimalMapTreeNodeView; /** * {@link FlexTreeField} that stores a sequence of children. diff --git a/packages/dds/tree/src/feature-libraries/flex-tree/index.ts b/packages/dds/tree/src/feature-libraries/flex-tree/index.ts index 92cae16dde2d..a076c4f903c7 100644 --- a/packages/dds/tree/src/feature-libraries/flex-tree/index.ts +++ b/packages/dds/tree/src/feature-libraries/flex-tree/index.ts @@ -17,6 +17,8 @@ export { FlexTreeEntityKind, isFlexTreeNode, flexTreeSlot, + type FlexibleNodeContent, + type FlexibleFieldContent, } from "./flexTreeTypes.js"; export { @@ -32,6 +34,7 @@ export { type FlexTreeHydratedContext, Context, ContextSlot, + type FlexTreeHydratedContextMinimal, } from "./context.js"; export { type FlexTreeNodeEvents } from "./treeEvents.js"; diff --git a/packages/dds/tree/src/feature-libraries/flex-tree/lazyField.ts b/packages/dds/tree/src/feature-libraries/flex-tree/lazyField.ts index cfbec7ed14c6..dc4cbf979bcb 100644 --- a/packages/dds/tree/src/feature-libraries/flex-tree/lazyField.ts +++ b/packages/dds/tree/src/feature-libraries/flex-tree/lazyField.ts @@ -43,6 +43,8 @@ import { type FlexTreeSequenceField, type FlexTreeTypedField, type FlexTreeUnknownUnboxed, + type FlexibleFieldContent, + type FlexibleNodeContent, TreeStatus, flexTreeMarker, flexTreeSlot, @@ -263,7 +265,7 @@ export class LazySequence extends LazyField implements FlexTreeSequenceField { return this.map((x) => x); } - public editor: SequenceFieldEditBuilder = { + public editor: SequenceFieldEditBuilder = { insert: (index, newContent) => { this.sequenceEditor().insert(index, cursorForMapTreeField(newContent)); }, @@ -279,7 +281,7 @@ export class LazySequence extends LazyField implements FlexTreeSequenceField { } export class LazyValueField extends LazyField implements FlexTreeRequiredField { - public editor: ValueFieldEditBuilder = { + public editor: ValueFieldEditBuilder = { set: (newContent) => { this.valueFieldEditor().set(cursorForMapTreeField([newContent])); }, diff --git a/packages/dds/tree/src/feature-libraries/index.ts b/packages/dds/tree/src/feature-libraries/index.ts index e89e19040f54..436a0174dfb7 100644 --- a/packages/dds/tree/src/feature-libraries/index.ts +++ b/packages/dds/tree/src/feature-libraries/index.ts @@ -174,6 +174,9 @@ export { treeStatusFromAnchorCache, indexForAt, FlexTreeEntityKind, + type FlexibleNodeContent, + type FlexibleFieldContent, + type FlexTreeHydratedContextMinimal, } from "./flex-tree/index.js"; export { TreeCompressionStrategy } from "./treeCompressionUtils.js"; diff --git a/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts b/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts index f47da900c48d..4e17a08b0f46 100644 --- a/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts +++ b/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts @@ -12,6 +12,7 @@ import { type FieldKey, type ITreeCursor, type MapTree, + type NodeData, aboveRootPlaceholder, detachedFieldAsKey, mapCursorField, @@ -21,40 +22,76 @@ import { import { type CursorAdapter, type CursorWithNode, + type Field, stackTreeFieldCursor, stackTreeNodeCursor, } from "./treeCursorUtils.js"; +import type { requireAssignableTo } from "../util/index.js"; + +export interface MapTreeNodeViewGeneric extends NodeData { + readonly fields: Pick< + ReadonlyMap>, + typeof Symbol.iterator | "get" | "size" | "keys" + >; +} + +export type MapTreeFieldViewGeneric = Pick< + readonly TNode[], + typeof Symbol.iterator | "length" +>; + +export interface MinimalMapTreeNodeView + extends MapTreeNodeViewGeneric {} + +{ + type _check1 = requireAssignableTo; + type _check2 = requireAssignableTo>; +} /** * Returns an {@link ITreeCursorSynchronous} in nodes mode for a single {@link MapTree}. */ -export function cursorForMapTreeNode(root: MapTree): CursorWithNode { - return stackTreeNodeCursor(adapter, root); +export function cursorForMapTreeNode>( + root: T, +): CursorWithNode { + const adapterTyped = adapter as unknown as CursorAdapter; + return stackTreeNodeCursor(adapterTyped, root); } /** * Returns an {@link ITreeCursorSynchronous} in fields mode for a MapTree field. */ -export function cursorForMapTreeField( - root: readonly MapTree[], +export function cursorForMapTreeField>( + root: readonly T[], detachedField: DetachedField = rootField, -): CursorWithNode { +): CursorWithNode { const key = detachedFieldAsKey(detachedField); - return stackTreeFieldCursor( - adapter, - { - type: aboveRootPlaceholder, - fields: new Map([[key, root]]), - }, - detachedField, - ); + const adapterTyped = adapter as unknown as CursorAdapter; + const dummyRoot: MapTreeNodeViewGeneric = { + type: aboveRootPlaceholder, + fields: new Map([[key, root]]), + }; + return stackTreeFieldCursor(adapterTyped, dummyRoot as T, detachedField); } -const adapter: CursorAdapter = { +const adapter: CursorAdapter = { 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 => { + const field = node.fields.get(key) as + | MinimalMapTreeNodeView[] + | Iterable + | 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[]; + }, }; /** diff --git a/packages/dds/tree/src/feature-libraries/treeCursorUtils.ts b/packages/dds/tree/src/feature-libraries/treeCursorUtils.ts index ff44f721dbd9..a2317e861bb0 100644 --- a/packages/dds/tree/src/feature-libraries/treeCursorUtils.ts +++ b/packages/dds/tree/src/feature-libraries/treeCursorUtils.ts @@ -72,6 +72,8 @@ export function stackTreeFieldCursor( return cursor; } +export type Field = Pick; + /** * Provides functionality to allow a {@link stackTreeNodeCursor} and {@link stackTreeFieldCursor} to implement cursors. */ @@ -91,10 +93,10 @@ export interface CursorAdapter { /** * @returns the child nodes for the given node and key. */ - getFieldFromNode(node: TNode, key: FieldKey): readonly TNode[]; + getFieldFromNode(node: TNode, key: FieldKey): Field; } -type SiblingsOrKey = readonly TNode[] | readonly FieldKey[]; +type SiblingsOrKey = Field | readonly FieldKey[]; /** * A class that satisfies part of the ITreeCursorSynchronous implementation. @@ -122,7 +124,7 @@ export abstract class SynchronousCursor { * 2. Support for cursors which are field cursors at the root. */ class StackCursor extends SynchronousCursor implements CursorWithNode { - public readonly [CursorMarker] = true; + public override readonly [CursorMarker] = true; /** * Might start at special root where fields are detached sequences. * @@ -149,14 +151,18 @@ class StackCursor extends SynchronousCursor implements CursorWithNode this.mode === CursorLocationType.Fields ? true : "must be in fields mode", ); - return this.siblings[this.index] as FieldKey; + // index is kept inbounds an an invariant of the class. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return (this.siblings as readonly FieldKey[])[this.index]!; } private getStackedFieldKey(height: number): FieldKey { assert(height % 2 === 1, 0x3b8 /* must field height */); const siblingStack = this.siblingStack[height] ?? oob(); const indexStack = this.indexStack[height] ?? oob(); - return siblingStack[indexStack] as FieldKey; + // index is kept inbounds an an invariant of the class. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return (siblingStack as readonly FieldKey[])[indexStack]!; } private getStackedNodeIndex(height: number): number { @@ -166,9 +172,10 @@ class StackCursor extends SynchronousCursor implements CursorWithNode)[index]!; } public getFieldLength(): number { @@ -358,12 +365,13 @@ class StackCursor extends SynchronousCursor implements CursorWithNode)[this.index]!; } - private getField(): readonly TNode[] { + private getField(): Field { // assert(this.mode === CursorLocationType.Fields, "can only get field when in fields"); const parent = this.getStackedNode(this.indexStack.length - 1); const key: FieldKey = this.getFieldKey(); diff --git a/packages/dds/tree/src/shared-tree/treeAlpha.ts b/packages/dds/tree/src/shared-tree/treeAlpha.ts index f04e7172b479..dd17e5973307 100644 --- a/packages/dds/tree/src/shared-tree/treeAlpha.ts +++ b/packages/dds/tree/src/shared-tree/treeAlpha.ts @@ -58,7 +58,8 @@ import { import { independentInitializedView, type ViewContent } from "./independentView.js"; import { SchematizingSimpleTreeView, ViewSlot } from "./schematizingTreeView.js"; import { currentVersion } from "../codec/index.js"; -import { createFromMapTree } from "../simple-tree/index.js"; +// eslint-disable-next-line import/no-internal-modules +import { createTreeNodeFromInner } from "../simple-tree/core/treeNodeKernel.js"; const identifier: TreeIdentifierUtils = (node: TreeNode): string | undefined => { const nodeIdentifier = getIdentifierFromNode(node, "uncompressed"); @@ -368,7 +369,7 @@ export const TreeAlpha: TreeAlpha = { : TreeNode | TreeLeafValue | undefined > { const mapTree = mapTreeFromNodeData(data as InsertableField, schema); - const result = mapTree === undefined ? undefined : createFromMapTree(schema, mapTree); + const result = mapTree === undefined ? undefined : createTreeNodeFromInner(mapTree); return result as Unhydrated< TSchema extends ImplicitFieldSchema ? TreeFieldFromImplicitField diff --git a/packages/dds/tree/src/simple-tree/api/create.ts b/packages/dds/tree/src/simple-tree/api/create.ts index 25ffa10f033b..c27ab7ff1cf9 100644 --- a/packages/dds/tree/src/simple-tree/api/create.ts +++ b/packages/dds/tree/src/simple-tree/api/create.ts @@ -3,19 +3,11 @@ * Licensed under the MIT License. */ -import { assert } from "@fluidframework/core-utils/internal"; +import { assert, fail } from "@fluidframework/core-utils/internal"; -import type { - ExclusiveMapTree, - ITreeCursorSynchronous, - SchemaAndPolicy, -} from "../../core/index.js"; +import type { ITreeCursorSynchronous, SchemaAndPolicy } from "../../core/index.js"; import type { ImplicitFieldSchema, TreeFieldFromImplicitField } from "../schemaTypes.js"; -import { - getOrCreateNodeFromInnerNode, - UnhydratedFlexTreeNode, - type Unhydrated, -} from "../core/index.js"; +import type { Unhydrated } from "../core/index.js"; import { defaultSchemaPolicy, inSchemaOrThrow, @@ -57,22 +49,7 @@ export function createFromCursor( assert(mapTrees.length === 1, 0xa11 /* unexpected field length */); // Length asserted above, so this is safe. This assert is done instead of checking for undefined after indexing to ensure a length greater than 1 also errors. // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const mapTree = mapTrees[0]!; - return createFromMapTree(schema, mapTree); -} - -/** - * Creates an unhydrated simple-tree field from an ExclusiveMapTree. - */ -export function createFromMapTree( - schema: TSchema, - mapTree: ExclusiveMapTree, -): Unhydrated> { - const mapTreeNode = UnhydratedFlexTreeNode.getOrCreate( - getUnhydratedContext(schema), - mapTree, - ); + const _mapTree = mapTrees[0]!; - const result = getOrCreateNodeFromInnerNode(mapTreeNode); - return result as Unhydrated>; + return fail("TODO"); } diff --git a/packages/dds/tree/src/simple-tree/api/index.ts b/packages/dds/tree/src/simple-tree/api/index.ts index 68153f70adc2..71cb5e7dcc69 100644 --- a/packages/dds/tree/src/simple-tree/api/index.ts +++ b/packages/dds/tree/src/simple-tree/api/index.ts @@ -52,10 +52,7 @@ export { getPropertyKeyFromStoredKey, getIdentifierFromNode, } from "./treeNodeApi.js"; -export { - createFromCursor, - createFromMapTree, -} from "./create.js"; +export { createFromCursor } from "./create.js"; export { type JsonSchemaId, type JsonSchemaType, diff --git a/packages/dds/tree/src/simple-tree/api/schemaFactory.ts b/packages/dds/tree/src/simple-tree/api/schemaFactory.ts index b6642607234f..28233dfdaa0d 100644 --- a/packages/dds/tree/src/simple-tree/api/schemaFactory.ts +++ b/packages/dds/tree/src/simple-tree/api/schemaFactory.ts @@ -8,7 +8,6 @@ import { assert, unreachableCase } from "@fluidframework/core-utils/internal"; import { isFluidHandle } from "@fluidframework/runtime-utils/internal"; import type { TreeValue } from "../../core/index.js"; -import type { NodeIdentifierManager } from "../../feature-libraries/index.js"; // This import is required for intellisense in @link doc comments on mouseover in VSCode. // eslint-disable-next-line unused-imports/no-unused-imports, @typescript-eslint/no-unused-vars import type { TreeAlpha } from "../../shared-tree/index.js"; @@ -24,6 +23,7 @@ import type { TreeNodeSchemaClass, TreeNodeSchemaNonClass, TreeNodeSchemaBoth, + UnhydratedFlexTreeNode, } from "../core/index.js"; import { isLazy } from "../flexList.js"; import { @@ -64,6 +64,10 @@ import { import { createFieldSchemaUnsafe } from "./schemaFactoryRecursive.js"; import type { System_Unsafe, FieldSchemaAlphaUnsafe } from "./typesUnsafe.js"; +import type { IIdCompressor } from "@fluidframework/id-compressor"; +import { createIdCompressor } from "@fluidframework/id-compressor/internal"; +import type { FlexTreeHydratedContextMinimal } from "../../feature-libraries/index.js"; +import { mapTreeFromNodeData } from "../toMapTree.js"; /** * Gets the leaf domain schema compatible with a given {@link TreeValue}. @@ -1080,8 +1084,6 @@ export class SchemaFactory< * * - A compressed form of the identifier can be accessed at runtime via the {@link TreeNodeApi.shortId|Tree.shortId()} API. * - * - It will not be present in the object's iterable properties until explicitly read or until having been inserted into a tree. - * * However, a user may alternatively supply their own string as the identifier if desired (for example, if importing identifiers from another system). * In that case, if the user requires it to be unique, it is up to them to ensure uniqueness. * User-supplied identifiers may be read immediately, even before insertion into the tree. @@ -1090,10 +1092,19 @@ export class SchemaFactory< */ public get identifier(): FieldSchema { const defaultIdentifierProvider: DefaultProvider = getDefaultProvider( - (nodeKeyManager: NodeIdentifierManager) => { - return nodeKeyManager.stabilizeNodeIdentifier( - nodeKeyManager.generateLocalNodeIdentifier(), - ); + ( + context: FlexTreeHydratedContextMinimal | "UseGlobalContext", + ): UnhydratedFlexTreeNode => { + const id = + context === "UseGlobalContext" + ? globalIdentifierAllocator.decompress( + globalIdentifierAllocator.generateCompressedId(), + ) + : context.nodeKeyManager.stabilizeNodeIdentifier( + context.nodeKeyManager.generateLocalNodeIdentifier(), + ); + + return mapTreeFromNodeData(id, this.string); }, ); return createFieldSchema(FieldKind.Identifier, this.string, { @@ -1266,3 +1277,5 @@ export function structuralName( } return `${collectionName}<${inner}>`; } + +const globalIdentifierAllocator: IIdCompressor = createIdCompressor(); diff --git a/packages/dds/tree/src/simple-tree/api/treeNodeApi.ts b/packages/dds/tree/src/simple-tree/api/treeNodeApi.ts index a436013eec92..13052aee225d 100644 --- a/packages/dds/tree/src/simple-tree/api/treeNodeApi.ts +++ b/packages/dds/tree/src/simple-tree/api/treeNodeApi.ts @@ -34,12 +34,11 @@ import { type TreeNode, tryGetTreeNodeSchema, getOrCreateNodeFromInnerNode, - UnhydratedFlexTreeNode, typeSchemaSymbol, getOrCreateInnerNode, } from "../core/index.js"; import type { TreeChangeEvents } from "./treeChangeEvents.js"; -import { lazilyAllocateIdentifier, isObjectNodeSchema } from "../node-kinds/index.js"; +import { isObjectNodeSchema } from "../node-kinds/index.js"; /** * Provides various functions for analyzing {@link TreeNode}s. @@ -304,12 +303,6 @@ export function getIdentifierFromNode( case 1: { const key = identifierFieldKeys[0] ?? oob(); const identifier = flexNode.tryGetField(key)?.boxedAt(0); - if (flexNode instanceof UnhydratedFlexTreeNode) { - if (identifier === undefined) { - return lazilyAllocateIdentifier(flexNode, key); - } - return identifier.value as string; - } assert( identifier?.context.isHydrated() === true, 0xa27 /* Expected hydrated identifier */, diff --git a/packages/dds/tree/src/simple-tree/core/index.ts b/packages/dds/tree/src/simple-tree/core/index.ts index e542ee8a7130..c150e4ab9173 100644 --- a/packages/dds/tree/src/simple-tree/core/index.ts +++ b/packages/dds/tree/src/simple-tree/core/index.ts @@ -39,6 +39,5 @@ export { getOrCreateNodeFromInnerNode } from "./getOrCreateNode.js"; export { UnhydratedFlexTreeNode, UnhydratedTreeSequenceField, - tryUnhydratedFlexTreeNode, UnhydratedContext, } from "./unhydratedFlexTree.js"; diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index d22a059a662c..f49df3f9a6d6 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -12,12 +12,11 @@ import { type AnchorEvents, type AnchorNode, EmptyKey, - type ExclusiveMapTree, type FieldKey, type FieldKindIdentifier, forbiddenFieldKindIdentifier, type ITreeCursorSynchronous, - type MapTree, + type NodeData, type NormalizedFieldUpPath, type SchemaPolicy, type TreeNodeSchemaIdentifier, @@ -41,21 +40,31 @@ import { type FlexFieldKind, FieldKinds, type SequenceFieldEditBuilder, + type OptionalFieldEditBuilder, + type ValueFieldEditBuilder, + type FlexibleNodeContent, + type FlexTreeHydratedContextMinimal, cursorForMapTreeNode, } from "../../feature-libraries/index.js"; -import { brand, getOrCreate, mapIterable } from "../../util/index.js"; +import { brand, getOrCreate } from "../../util/index.js"; import type { Context } from "./context.js"; +import type { ContextualFieldProvider } from "../schemaTypes.js"; +import type { + MapTreeFieldViewGeneric, + MapTreeNodeViewGeneric, + // eslint-disable-next-line import/no-internal-modules +} from "../../feature-libraries/mapTreeCursor.js"; interface UnhydratedTreeSequenceFieldEditBuilder - extends SequenceFieldEditBuilder { + extends SequenceFieldEditBuilder { /** * Issues a change which removes `count` elements starting at the given `index`. * @param index - The index of the first removed element. * @param count - The number of elements to remove. * @returns the MapTrees that were removed */ - remove(index: number, count: number): ExclusiveMapTree[]; + remove(index: number, count: number): UnhydratedFlexTreeNode[]; } type UnhydratedFlexTreeNodeEvents = Pick; @@ -74,14 +83,12 @@ interface LocationInField { * * Create a `UnhydratedFlexTreeNode` by calling {@link getOrCreate}. */ -export class UnhydratedFlexTreeNode implements FlexTreeNode { - public get schema(): TreeNodeSchemaIdentifier { - return this.mapTree.type; - } - +export class UnhydratedFlexTreeNode + implements FlexTreeNode, MapTreeNodeViewGeneric +{ public get storedSchema(): TreeNodeStoredSchema { return ( - this.context.schema.nodeSchema.get(this.mapTree.type) ?? fail(0xb46 /* missing schema */) + this.context.schema.nodeSchema.get(this.data.type) ?? fail(0xb46 /* missing schema */) ); } @@ -92,19 +99,6 @@ export class UnhydratedFlexTreeNode implements FlexTreeNode { return this._events; } - /** - * Create a {@link UnhydratedFlexTreeNode} that wraps the given {@link MapTree}, or get the node that already exists for that {@link MapTree} if there is one. - * @param nodeSchema - the {@link FlexTreeNodeSchema | schema} that the node conforms to - * @param mapTree - the {@link MapTree} containing the data for this node. - * @remarks It must conform to the `nodeSchema`. - */ - public static getOrCreate( - context: Context, - mapTree: ExclusiveMapTree, - ): UnhydratedFlexTreeNode { - return nodeCache.get(mapTree) ?? new UnhydratedFlexTreeNode(context, mapTree, undefined); - } - public get context(): FlexTreeContext { return this.simpleContext.flexContext; } @@ -118,26 +112,31 @@ export class UnhydratedFlexTreeNode implements FlexTreeNode { * Instead, it should always be acquired via {@link getOrCreateNodeFromInnerNode}. */ public constructor( + public readonly data: NodeData, + public readonly fields: Map, public readonly simpleContext: Context, - /** The underlying {@link MapTree} that this `UnhydratedFlexTreeNode` reads its data from */ - public readonly mapTree: ExclusiveMapTree, private location = unparentedLocation, ) { - assert(!nodeCache.has(mapTree), 0x98b /* A node already exists for the given MapTree */); - nodeCache.set(mapTree, this); - - // Fully demand the tree to ensure that parent pointers are present and accurate on all nodes. - // When a UnhydratedFlexTreeNode is constructed, its MapTree may contain nodes (anywhere below) that map (via the `nodeCache`) to pre-existing UnhydratedFlexTreeNodes. - // Put another way, for a given MapTree, some ancestor UnhydratedFlexTreeNode can be created after any number of its descendant UnhydratedFlexTreeNodes already exist. - // In such a case, the spine of nodes between the descendant and ancestor need to exist in order for the ancestor to be able to walk upwards via the `parentField` property. - // This needs to happen for all UnhydratedFlexTreeNodes that are descendants of the ancestor UnhydratedFlexTreeNode. - // Demanding the entire tree is overkill to solve this problem since not all descendant MapTree nodes will have corresponding UnhydratedFlexTreeNodes. - // However, demanding the full tree also lets us eagerly validate that there are no duplicate MapTrees (i.e. same MapTree object) anywhere in the tree. - this.walkTree(); + for (const [_key, field] of this.fields) { + field.parent = this; + } } public get type(): TreeNodeSchemaIdentifier { - return this.mapTree.type; + return this.data.type; + } + + public get schema(): TreeNodeSchemaIdentifier { + return this.data.type; + } + + private getOrCreateField(key: FieldKey): UnhydratedFlexTreeField { + return getOrCreate(this.fields, key, () => { + const stored = this.storedSchema.getFieldSchema(key).kind; + const field = createField(this.simpleContext, stored, key, []); + field.parent = this; + return field; + }); } /** @@ -188,44 +187,37 @@ export class UnhydratedFlexTreeNode implements FlexTreeNode { } public borrowCursor(): ITreeCursorSynchronous { - return cursorForMapTreeNode(this.mapTree); + return cursorForMapTreeNode>(this); } public tryGetField(key: FieldKey): UnhydratedFlexTreeField | undefined { - const field = this.mapTree.fields.get(key); + const field = this.fields.get(key); // Only return the field if it is not empty, in order to fulfill the contract of `tryGetField`. if (field !== undefined && field.length > 0) { - return getOrCreateField(this, key, this.storedSchema.getFieldSchema(key).kind, () => - this.emitChangedEvent(key), - ); + return field; } } - public getBoxed(key: string): FlexTreeField { + public getBoxed(key: string): UnhydratedFlexTreeField { const fieldKey: FieldKey = brand(key); - return getOrCreateField( - this, - fieldKey, - this.storedSchema.getFieldSchema(fieldKey).kind, - () => this.emitChangedEvent(fieldKey), - ); + return this.getOrCreateField(fieldKey); } public boxedIterator(): IterableIterator { - return mapIterable(this.mapTree.fields.entries(), ([key]) => - getOrCreateField(this, key, this.storedSchema.getFieldSchema(key).kind, () => - this.emitChangedEvent(key), - ), - ); + return [...this.fields.values()] + .filter((field) => field.children.length > 0) + [Symbol.iterator](); } public keys(): IterableIterator { - // TODO: how this should handle missing defaults (and empty keys if they end up being allowed) needs to be determined. - return this.mapTree.fields.keys(); + return [...this.fields.entries()] + .filter((kv) => kv[1].children.length > 0) + .map((kv) => kv[0]) + [Symbol.iterator](); } public get value(): Value { - return this.mapTree.value; + return this.data.value; } public get anchorNode(): AnchorNode { @@ -234,32 +226,7 @@ export class UnhydratedFlexTreeNode implements FlexTreeNode { return fail(0xb47 /* UnhydratedFlexTreeNode does not implement anchorNode */); } - private walkTree(): void { - for (const [key, mapTrees] of this.mapTree.fields) { - const field = getOrCreateField( - this, - key, - this.storedSchema.getFieldSchema(key).kind, - () => this.emitChangedEvent(key), - ); - for (let index = 0; index < field.length; index++) { - const child = getOrCreateChild(this.simpleContext, mapTrees[index] ?? oob(), { - parent: field, - index, - }); - // These next asserts detect the case where `getOrCreateChild` gets a cache hit of a different node than the one we're trying to create - assert(child.location !== undefined, 0x98d /* Expected node to have parent */); - assert( - child.location.parent.parent === this, - 0x98e /* Node may not be multi-parented */, - ); - assert(child.location.index === index, 0x98f /* Node may not be multi-parented */); - child.walkTree(); - } - } - } - - private emitChangedEvent(key: FieldKey): void { + public emitChangedEvent(key: FieldKey): void { this._events.emit("childrenChangedAfterBatch", { changedFields: new Set([key]) }); } } @@ -323,59 +290,72 @@ const unparentedLocation: LocationInField = { index: -1, }; -class UnhydratedFlexTreeField implements FlexTreeField { +export class UnhydratedFlexTreeField + implements FlexTreeField, MapTreeFieldViewGeneric +{ public [flexTreeMarker] = FlexTreeEntityKind.Field as const; public get context(): FlexTreeContext { return this.simpleContext.flexContext; } + public parent: UnhydratedFlexTreeNode | undefined = undefined; + public constructor( public readonly simpleContext: Context, public readonly schema: FieldKindIdentifier, public readonly key: FieldKey, - public readonly parent: UnhydratedFlexTreeNode, - public readonly onEdit?: () => void, + private lazyChildren: UnhydratedFlexTreeNode[] | ContextualFieldProvider, ) { - const fieldKeyCache = getFieldKeyCache(parent); - assert(!fieldKeyCache.has(key), 0x990 /* A field already exists for the given MapTrees */); - fieldKeyCache.set(key, this); - // When this field is created (which only happens one time, because it is cached), all the children become parented for the first time. // "Adopt" each child by updating its parent information to point to this field. - for (const [i, mapTree] of this.mapTrees.entries()) { - const mapTreeNodeChild = nodeCache.get(mapTree); - if (mapTreeNodeChild !== undefined) { - if (mapTreeNodeChild.parentField !== unparentedLocation) { - throw new UsageError("A node may not be in more than one place in the tree"); - } - mapTreeNodeChild.adoptBy(this, i); + if (Array.isArray(lazyChildren)) { + for (const [i, child] of lazyChildren.entries()) { + child.adoptBy(this, i); } } } - public get mapTrees(): readonly ExclusiveMapTree[] { - return this.parent.mapTree.fields.get(this.key) ?? []; + private getPendingDefault(): ContextualFieldProvider | undefined { + return !Array.isArray(this.lazyChildren) ? this.lazyChildren : undefined; + } + + /** + * Populate pending default (if present) using the provided context. + * @remarks + * This apply to just this field: caller will likely want to recursively walk the tree. + */ + public fillPendingDefaults(context: FlexTreeHydratedContextMinimal): void { + const provider = this.getPendingDefault(); + if (provider) { + const content = provider(context); + this.lazyChildren = content === undefined ? [] : [content]; + } + } + + public get pendingDefault(): boolean { + return this.getPendingDefault() !== undefined; + } + + public get children(): UnhydratedFlexTreeNode[] { + const provider = this.getPendingDefault(); + if (provider) { + const content = provider("UseGlobalContext"); + this.lazyChildren = content === undefined ? [] : [content]; + } + return this.lazyChildren as UnhydratedFlexTreeNode[]; } public get length(): number { - return this.mapTrees.length; + return this.children.length; } public is(kind: TKind2): this is FlexTreeTypedField { return this.schema === kind.identifier; } - public boxedIterator(): IterableIterator { - return this.mapTrees - .map( - (m, index) => - getOrCreateChild(this.simpleContext, m, { - parent: this, - index, - }) as FlexTreeNode, - ) - .values(); + public boxedIterator(): IterableIterator { + return this.children[Symbol.iterator](); } public boxedAt(index: number): FlexTreeNode | undefined { @@ -383,13 +363,12 @@ class UnhydratedFlexTreeField implements FlexTreeField { if (i === undefined) { return undefined; } - const m = this.mapTrees[i]; - if (m !== undefined) { - return getOrCreateChild(this.simpleContext, m, { - parent: this, - index: i, - }) as FlexTreeNode; - } + const m = this.children[i]; + return m; + } + + public [Symbol.iterator](): IterableIterator { + return this.boxedIterator(); } /** @@ -400,16 +379,11 @@ class UnhydratedFlexTreeField implements FlexTreeField { * @remarks All edits to the field (i.e. mutations of the field's MapTrees) should be directed through this function. * This function ensures that the parent MapTree has no empty fields (which is an invariant of `MapTree`) after the mutation. */ - protected edit(edit: (mapTrees: ExclusiveMapTree[]) => void | ExclusiveMapTree[]): void { - const oldMapTrees = this.parent.mapTree.fields.get(this.key) ?? []; - const newMapTrees = edit(oldMapTrees) ?? oldMapTrees; - if (newMapTrees.length > 0) { - this.parent.mapTree.fields.set(this.key, newMapTrees); - } else { - this.parent.mapTree.fields.delete(this.key); - } - - this.onEdit?.(); + protected edit( + edit: (field: UnhydratedFlexTreeNode[]) => void | UnhydratedFlexTreeNode[], + ): void { + this.lazyChildren = edit(this.children) ?? this.children; + this.parent?.emitChangedEvent(this.key); } public getFieldPath(): NormalizedFieldUpPath { @@ -417,32 +391,25 @@ class UnhydratedFlexTreeField implements FlexTreeField { } /** Unboxes leaf nodes to their values */ - protected unboxed(index: number): FlexTreeUnknownUnboxed { - const mapTree: ExclusiveMapTree = this.mapTrees[index] ?? oob(); - const value = mapTree.value; - if (value !== undefined) { - return value; - } - - return getOrCreateChild(this.simpleContext, mapTree, { parent: this, index }); + protected unboxed(index: number): UnhydratedFlexTreeNode { + return this.children[index] ?? oob(); } } -class EagerMapTreeOptionalField +export class EagerMapTreeOptionalField extends UnhydratedFlexTreeField implements FlexTreeOptionalField { public readonly editor = { - set: (newContent: ExclusiveMapTree | undefined): void => { + set: (newContent: FlexibleNodeContent | undefined): void => { // If the new content is a UnhydratedFlexTreeNode, it needs to have its parent pointer updated if (newContent !== undefined) { - nodeCache.get(newContent)?.adoptBy(this, 0); + assert(newContent instanceof UnhydratedFlexTreeNode, "Expected unhydrated node"); + newContent.adoptBy(this, 0); } // If the old content is a UnhydratedFlexTreeNode, it needs to have its parent pointer unset - const oldContent = this.mapTrees[0]; - if (oldContent !== undefined) { - nodeCache.get(oldContent)?.adoptBy(undefined); - } + const oldContent = this.children[0]; + oldContent?.adoptBy(undefined); this.edit((mapTrees) => { if (newContent !== undefined) { @@ -452,10 +419,11 @@ class EagerMapTreeOptionalField } }); }, - }; + } satisfies OptionalFieldEditBuilder & + ValueFieldEditBuilder; public get content(): FlexTreeUnknownUnboxed | undefined { - const value = this.mapTrees[0]; + const value = this.children[0]; if (value !== undefined) { return this.unboxed(0); } @@ -464,7 +432,7 @@ class EagerMapTreeOptionalField } } -class EagerMapTreeRequiredField +export class EagerMapTreeRequiredField extends EagerMapTreeOptionalField implements FlexTreeRequiredField { @@ -487,7 +455,8 @@ export class UnhydratedTreeSequenceField for (let i = 0; i < newContent.length; i++) { const c = newContent[i]; assert(c !== undefined, 0xa0a /* Unexpected sparse array content */); - nodeCache.get(c)?.adoptBy(this, index + i); + c.adoptBy(this, index + i); + // TODO: don't the indexes of existing children after the insert (or remove) need to be updated? } this.edit((mapTrees) => { if (newContent.length < 1000) { @@ -499,13 +468,13 @@ export class UnhydratedTreeSequenceField } }); }, - remove: (index, count): ExclusiveMapTree[] => { + remove: (index, count): UnhydratedFlexTreeNode[] => { for (let i = index; i < index + count; i++) { - const c = this.mapTrees[i]; + const c = this.children[i]; assert(c !== undefined, 0xa0b /* Unexpected sparse array */); - nodeCache.get(c)?.adoptBy(undefined); + c.adoptBy(undefined); } - let removed: ExclusiveMapTree[] | undefined; + let removed: UnhydratedFlexTreeNode[] | undefined; this.edit((mapTrees) => { removed = mapTrees.splice(index, count); }); @@ -523,91 +492,37 @@ export class UnhydratedTreeSequenceField public map(callbackfn: (value: FlexTreeUnknownUnboxed, index: number) => U): U[] { return Array.from(this, callbackfn); } - - public *[Symbol.iterator](): IterableIterator { - for (const [i] of this.mapTrees.entries()) { - yield this.unboxed(i); - } - } } // #endregion Fields -// #region Caching and unboxing utilities - -const nodeCache = new WeakMap(); -/** Node Parent -\> Field Key -\> Field */ -const fieldCache = new WeakMap< - UnhydratedFlexTreeNode, - Map ->(); -function getFieldKeyCache( - parent: UnhydratedFlexTreeNode, -): WeakMap { - return getOrCreate(fieldCache, parent, () => new Map()); -} - -/** - * If there exists a {@link UnhydratedFlexTreeNode} for the given {@link MapTree}, returns it, otherwise returns `undefined`. - * @remarks {@link UnhydratedFlexTreeNode | UnhydratedFlexTreeNodes} are created via {@link getOrCreateNodeFromInnerNode}. - */ -export function tryUnhydratedFlexTreeNode( - mapTree: MapTree, -): UnhydratedFlexTreeNode | undefined { - return nodeCache.get(mapTree); -} - -/** Helper for creating a `UnhydratedFlexTreeNode` given the parent field (e.g. when "walking down") */ -function getOrCreateChild( - context: Context, - mapTree: ExclusiveMapTree, - parent: LocationInField | undefined, -): UnhydratedFlexTreeNode { - const cached = nodeCache.get(mapTree); - if (cached !== undefined) { - return cached; - } - - return new UnhydratedFlexTreeNode(context, mapTree, parent); -} - -/** Creates a field with the given attributes, or returns a cached field if there is one */ -function getOrCreateField( - parent: UnhydratedFlexTreeNode, - key: FieldKey, - schema: FieldKindIdentifier, - onEdit?: () => void, +/** Creates a field with the given attributes */ +export function createField( + ...args: [ + Context, + FieldKindIdentifier, + FieldKey, + UnhydratedFlexTreeNode[] | ContextualFieldProvider, + ] ): UnhydratedFlexTreeField { - const cached = getFieldKeyCache(parent).get(key); - if (cached !== undefined) { - return cached; - } - - if ( - schema === FieldKinds.required.identifier || - schema === FieldKinds.identifier.identifier - ) { - return new EagerMapTreeRequiredField(parent.simpleContext, schema, key, parent, onEdit); - } - - if (schema === FieldKinds.optional.identifier) { - return new EagerMapTreeOptionalField(parent.simpleContext, schema, key, parent, onEdit); - } - - if (schema === FieldKinds.sequence.identifier) { - return new UnhydratedTreeSequenceField(parent.simpleContext, schema, key, parent, onEdit); + switch (args[1]) { + case FieldKinds.required.identifier: + case FieldKinds.identifier.identifier: + return new EagerMapTreeRequiredField(...args); + + case FieldKinds.optional.identifier: + return new EagerMapTreeOptionalField(...args); + + case FieldKinds.sequence.identifier: + return new UnhydratedTreeSequenceField(...args); + case FieldKinds.forbidden.identifier: + // TODO: this seems to used by unknown optional fields. They should probably use "optional" not "Forbidden" schema. + return new UnhydratedFlexTreeField(...args); + default: + return fail("unsupported field kind"); } - - // TODO: this seems to used by unknown optional fields. They should probably use "optional" not "Forbidden" schema. - if (schema === FieldKinds.forbidden.identifier) { - return new UnhydratedFlexTreeField(parent.simpleContext, schema, key, parent, onEdit); - } - - return fail("unsupported field kind"); } -// #endregion Caching and unboxing utilities - export function unsupportedUsageError(message?: string): Error { return new UsageError( `${ diff --git a/packages/dds/tree/src/simple-tree/index.ts b/packages/dds/tree/src/simple-tree/index.ts index 134dd51ba5f8..b7c32c83b192 100644 --- a/packages/dds/tree/src/simple-tree/index.ts +++ b/packages/dds/tree/src/simple-tree/index.ts @@ -120,7 +120,6 @@ export { type FixRecursiveRecursionLimit, schemaStatics, type TreeChangeEvents, - createFromMapTree, } from "./api/index.js"; export type { SimpleTreeSchema, diff --git a/packages/dds/tree/src/simple-tree/node-kinds/array/arrayNode.ts b/packages/dds/tree/src/simple-tree/node-kinds/array/arrayNode.ts index cf765466a005..67a07b9beec0 100644 --- a/packages/dds/tree/src/simple-tree/node-kinds/array/arrayNode.ts +++ b/packages/dds/tree/src/simple-tree/node-kinds/array/arrayNode.ts @@ -6,8 +6,9 @@ import { Lazy, oob, fail } from "@fluidframework/core-utils/internal"; import { UsageError } from "@fluidframework/telemetry-utils/internal"; -import { EmptyKey, type ExclusiveMapTree } from "../../../core/index.js"; +import { EmptyKey } from "../../../core/index.js"; import { + type FlexibleFieldContent, type FlexTreeNode, type FlexTreeSequenceField, isFlexTreeNode, @@ -37,14 +38,12 @@ import { getSimpleNodeSchemaFromInnerNode, getOrCreateInnerNode, type TreeNodeSchemaClass, -} from "../../core/index.js"; -import { type InsertableContent, mapTreeFromNodeData } from "../../toMapTree.js"; -import { prepareArrayContentForInsertion } from "../../prepareForInsertion.js"; -import { getKernel, - UnhydratedFlexTreeNode, + type UnhydratedFlexTreeNode, UnhydratedTreeSequenceField, } from "../../core/index.js"; +import { type InsertableContent, mapTreeFromNodeData } from "../../toMapTree.js"; +import { prepareArrayContentForInsertion } from "../../prepareForInsertion.js"; import { TreeNodeValid, type MostDerivedData } from "../../treeNodeValid.js"; import { getUnhydratedContext } from "../../createContext.js"; import type { System_Unsafe } from "../../api/index.js"; @@ -852,7 +851,7 @@ abstract class CustomArrayNodeBase super(input ?? []); } - #mapTreesFromFieldData(value: Insertable): ExclusiveMapTree[] { + #mapTreesFromFieldData(value: Insertable): FlexibleFieldContent { const sequenceField = getSequenceField(this); const content = value as readonly ( | InsertableContent @@ -1139,10 +1138,7 @@ export function arraySchema< instance: TreeNodeValid, input: T2, ): UnhydratedFlexTreeNode { - return UnhydratedFlexTreeNode.getOrCreate( - unhydratedContext, - mapTreeFromNodeData(input as object, this as unknown as ImplicitAllowedTypes), - ); + return mapTreeFromNodeData(input as object, this as unknown as ImplicitAllowedTypes); } public static get allowedTypesIdentifiers(): ReadonlySet { diff --git a/packages/dds/tree/src/simple-tree/node-kinds/index.ts b/packages/dds/tree/src/simple-tree/node-kinds/index.ts index e3b1a5f39fd4..b3bb722192a6 100644 --- a/packages/dds/tree/src/simple-tree/node-kinds/index.ts +++ b/packages/dds/tree/src/simple-tree/node-kinds/index.ts @@ -31,7 +31,6 @@ export { type InsertableObjectFromAnnotatedSchemaRecord, type InsertableObjectFromSchemaRecord, isObjectNodeSchema, - lazilyAllocateIdentifier, type ObjectFromSchemaRecord, ObjectNodeSchema, objectSchema, diff --git a/packages/dds/tree/src/simple-tree/node-kinds/map/mapNode.ts b/packages/dds/tree/src/simple-tree/node-kinds/map/mapNode.ts index 6759ae833b48..22a9e4930fba 100644 --- a/packages/dds/tree/src/simple-tree/node-kinds/map/mapNode.ts +++ b/packages/dds/tree/src/simple-tree/node-kinds/map/mapNode.ts @@ -5,6 +5,7 @@ import { Lazy } from "@fluidframework/core-utils/internal"; import type { + FlexibleNodeContent, FlexTreeNode, FlexTreeOptionalField, OptionalFieldEditBuilder, @@ -32,9 +33,9 @@ import { type TreeNode, typeSchemaSymbol, type Context, - UnhydratedFlexTreeNode, getOrCreateInnerNode, type InternalTreeNode, + type UnhydratedFlexTreeNode, } from "../../core/index.js"; import { mapTreeFromNodeData, @@ -44,7 +45,6 @@ import { import { prepareForInsertion } from "../../prepareForInsertion.js"; import { brand, count, type RestrictiveStringRecord } from "../../../util/index.js"; import { TreeNodeValid, type MostDerivedData } from "../../treeNodeValid.js"; -import type { ExclusiveMapTree } from "../../../core/index.js"; import { getUnhydratedContext } from "../../createContext.js"; import type { MapNodeCustomizableSchema, MapNodePojoEmulationSchema } from "./mapNodeTypes.js"; @@ -158,7 +158,7 @@ abstract class CustomMapNodeBase extends T return getOrCreateInnerNode(this); } - private editor(key: string): OptionalFieldEditBuilder { + private editor(key: string): OptionalFieldEditBuilder { const field = this.innerNode.getBoxed(brand(key)) as FlexTreeOptionalField; return field.editor; } @@ -271,9 +271,9 @@ export function mapSchema< instance: TreeNodeValid, input: T2, ): UnhydratedFlexTreeNode { - return UnhydratedFlexTreeNode.getOrCreate( - unhydratedContext, - mapTreeFromNodeData(input as FactoryContent, this as unknown as ImplicitAllowedTypes), + return mapTreeFromNodeData( + input as FactoryContent, + this as unknown as ImplicitAllowedTypes, ); } diff --git a/packages/dds/tree/src/simple-tree/node-kinds/object/index.ts b/packages/dds/tree/src/simple-tree/node-kinds/object/index.ts index 3e6590396634..7f65ed9a0112 100644 --- a/packages/dds/tree/src/simple-tree/node-kinds/object/index.ts +++ b/packages/dds/tree/src/simple-tree/node-kinds/object/index.ts @@ -8,7 +8,6 @@ export { type FieldHasDefault, type InsertableObjectFromAnnotatedSchemaRecord, type InsertableObjectFromSchemaRecord, - lazilyAllocateIdentifier, type ObjectFromSchemaRecord, objectSchema, setField, diff --git a/packages/dds/tree/src/simple-tree/node-kinds/object/objectNode.ts b/packages/dds/tree/src/simple-tree/node-kinds/object/objectNode.ts index ba695d1cc17d..bec8aaeef860 100644 --- a/packages/dds/tree/src/simple-tree/node-kinds/object/objectNode.ts +++ b/packages/dds/tree/src/simple-tree/node-kinds/object/objectNode.ts @@ -5,8 +5,6 @@ import { assert, Lazy, fail, debugAssert } from "@fluidframework/core-utils/internal"; import { UsageError } from "@fluidframework/telemetry-utils/internal"; -import type { IIdCompressor } from "@fluidframework/id-compressor"; -import { createIdCompressor } from "@fluidframework/id-compressor/internal"; import type { FieldKey, SchemaPolicy } from "../../../core/index.js"; import { @@ -16,7 +14,7 @@ import { type FlexTreeOptionalField, type FlexTreeRequiredField, } from "../../../feature-libraries/index.js"; -import { type RestrictiveStringRecord, type FlattenKeys, brand } from "../../../util/index.js"; +import type { RestrictiveStringRecord, FlattenKeys } from "../../../util/index.js"; import { type TreeNodeSchema, @@ -28,7 +26,7 @@ import { type InternalTreeNode, type TreeNode, type Context, - UnhydratedFlexTreeNode, + type UnhydratedFlexTreeNode, getOrCreateInnerNode, } from "../../core/index.js"; import { getUnhydratedContext } from "../../createContext.js"; @@ -59,7 +57,6 @@ import { import type { SimpleObjectFieldSchema } from "../../simpleSchema.js"; import { mapTreeFromNodeData, type InsertableContent } from "../../toMapTree.js"; import { TreeNodeValid, type MostDerivedData } from "../../treeNodeValid.js"; -import { stringSchema } from "../../leafNodeSchema.js"; /** * Generates the properties for an ObjectNode from its field schema object. @@ -201,37 +198,6 @@ function createFlexKeyMapping(fields: Record): Simp return keyMap; } -const globalIdentifierAllocator: IIdCompressor = createIdCompressor(); - -/** - * Modify `flexNode` to add a newly generated identifier under the given `storedKey`. - * @remarks - * This is used after checking if the user is trying to read an identifier field of an unhydrated node, but the identifier is not present. - * This means the identifier is an "auto-generated identifier", because otherwise it would have been supplied by the user at construction time and would have been successfully read just above. - * In this case, it is categorically impossible to provide an identifier (auto-generated identifiers can't be created until hydration/insertion time), so we emit an error. - * @privateRemarks - * TODO: this special case logic should move to the inner node (who's schema claims it has an identifier), rather than here, after we already read undefined out of a required field. - * TODO: unify this with a more general defaults mechanism. - */ -export function lazilyAllocateIdentifier( - flexNode: UnhydratedFlexTreeNode, - storedKey: FieldKey, -): string { - debugAssert(() => !flexNode.mapTree.fields.has(storedKey) || "Identifier field already set"); - const value = globalIdentifierAllocator.decompress( - globalIdentifierAllocator.generateCompressedId(), - ); - flexNode.mapTree.fields.set(storedKey, [ - { - type: brand(stringSchema.identifier), - value, - fields: new Map(), - }, - ]); - - return value; -} - /** * Creates a proxy handler for the given schema. * @@ -265,13 +231,6 @@ function createProxyHandler( return getTreeNodeForField(field); } - if ( - fieldInfo.schema.kind === FieldKind.Identifier && - flexNode instanceof UnhydratedFlexTreeNode - ) { - return lazilyAllocateIdentifier(flexNode, fieldInfo.storedKey); - } - return undefined; } @@ -495,10 +454,7 @@ export function objectSchema< instance: TreeNodeValid, input: T2, ): UnhydratedFlexTreeNode { - return UnhydratedFlexTreeNode.getOrCreate( - unhydratedContext, - mapTreeFromNodeData(input as object, this as unknown as ImplicitAllowedTypes), - ); + return mapTreeFromNodeData(input as object, this as unknown as ImplicitAllowedTypes); } protected static override constructorCached: MostDerivedData | undefined = undefined; diff --git a/packages/dds/tree/src/simple-tree/prepareForInsertion.ts b/packages/dds/tree/src/simple-tree/prepareForInsertion.ts index 83e3549a7e76..ec22a7155d23 100644 --- a/packages/dds/tree/src/simple-tree/prepareForInsertion.ts +++ b/packages/dds/tree/src/simple-tree/prepareForInsertion.ts @@ -4,10 +4,8 @@ */ import type { - ExclusiveMapTree, SchemaAndPolicy, IForestSubscription, - MapTree, UpPath, NodeIndex, FieldKey, @@ -19,6 +17,9 @@ import { getSchemaAndPolicy, type FlexTreeHydratedContext, FieldKinds, + type FlexTreeHydratedContextMinimal, + type FlexibleFieldContent, + type FlexibleNodeContent, } from "../feature-libraries/index.js"; import { normalizeFieldSchema, @@ -31,7 +32,7 @@ import { brand } from "../util/index.js"; import { getKernel, type TreeNode, - tryUnhydratedFlexTreeNode, + type UnhydratedFlexTreeNode, unhydratedFlexTreeNodeToTreeNode, } from "./core/index.js"; import { debugAssert, oob } from "@fluidframework/core-utils/internal"; @@ -50,7 +51,7 @@ export function prepareForInsertion( data: TIn, schema: ImplicitFieldSchema, destinationContext: FlexTreeContext, -): TIn extends undefined ? undefined : ExclusiveMapTree { +): TIn extends undefined ? undefined : FlexibleNodeContent { return prepareForInsertionContextless( data, schema, @@ -76,13 +77,9 @@ export function prepareArrayContentForInsertion( data: readonly InsertableContent[], schema: ImplicitAllowedTypes, destinationContext: FlexTreeContext, -): ExclusiveMapTree[] { - const mapTrees: ExclusiveMapTree[] = data.map((item) => - mapTreeFromNodeData( - item, - schema, - destinationContext.isHydrated() ? destinationContext.nodeKeyManager : undefined, - ), +): FlexibleFieldContent { + const mapTrees: UnhydratedFlexTreeNode[] = data.map((item) => + mapTreeFromNodeData(item, schema), ); const fieldSchema = convertField(normalizeFieldSchema(schema)); @@ -111,8 +108,8 @@ export function prepareForInsertionContextless | undefined, -): TIn extends undefined ? undefined : ExclusiveMapTree { - const mapTree = mapTreeFromNodeData(data, schema, hydratedData?.nodeKeyManager); +): TIn extends undefined ? undefined : FlexibleNodeContent { + const mapTree = mapTreeFromNodeData(data, schema); const contentArray = mapTree === undefined ? [] : [mapTree]; const fieldSchema = convertField(normalizeFieldSchema(schema)); @@ -129,16 +126,16 @@ export function prepareForInsertionContextless | undefined, + hydratedData: FlexTreeHydratedContextMinimal | undefined, fieldSchema: TreeFieldStoredSchema, - mapTrees: ExclusiveMapTree[], + mapTrees: readonly UnhydratedFlexTreeNode[], ): void { if (hydratedData !== undefined) { if (schemaAndPolicy.policy.validateSchema === true) { const maybeError = isFieldInSchema(mapTrees, fieldSchema, schemaAndPolicy); inSchemaOrThrow(maybeError); } - prepareContentForHydration(mapTrees, hydratedData.checkout.forest); + prepareContentForHydration(mapTrees, hydratedData.checkout.forest, hydratedData); } } @@ -185,8 +182,9 @@ const placeholderKey: DetachedField & FieldKey = brand("placeholder" as const); * See {@link extractFactoryContent} for more details. */ function prepareContentForHydration( - content: readonly MapTree[], + content: readonly UnhydratedFlexTreeNode[], forest: IForestSubscription, + context: FlexTreeHydratedContextMinimal, ): void { const batches: LocatedNodesBatch[] = []; for (const item of content) { @@ -199,39 +197,45 @@ function prepareContentForHydration( paths: [], }; batches.push(batch); - walkMapTree(item, batch.rootPath, (p, node) => { - batch.paths.push({ path: p, node }); - }); + walkMapTree( + item, + batch.rootPath, + (p, node) => { + batch.paths.push({ path: p, node }); + }, + context, + ); } scheduleHydration(batches, forest); } function walkMapTree( - mapTree: MapTree, + root: UnhydratedFlexTreeNode, path: UpPath, onVisitTreeNode: (path: UpPath, treeNode: TreeNode) => void, + context: FlexTreeHydratedContextMinimal, ): void { - if (tryUnhydratedFlexTreeNode(mapTree)?.parentField.parent.parent !== undefined) { + if (root.parentField.parent.parent !== undefined) { throw new UsageError( "Attempted to insert a node which is already under a parent. If this is desired, remove the node from its parent before inserting it elsewhere.", ); } - type Next = [path: UpPath, tree: MapTree]; + type Next = [path: UpPath, tree: UnhydratedFlexTreeNode]; const nexts: Next[] = []; - for (let next: Next | undefined = [path, mapTree]; next !== undefined; next = nexts.pop()) { - const [p, m] = next; - const mapTreeNode = tryUnhydratedFlexTreeNode(m); - if (mapTreeNode !== undefined) { - const treeNode = unhydratedFlexTreeNodeToTreeNode.get(mapTreeNode); + for (let next: Next | undefined = [path, root]; next !== undefined; next = nexts.pop()) { + const [p, node] = next; + if (node !== undefined) { + const treeNode = unhydratedFlexTreeNodeToTreeNode.get(node); if (treeNode !== undefined) { onVisitTreeNode(p, treeNode); } } - for (const [key, field] of m.fields) { - for (const [i, child] of field.entries()) { + for (const [key, field] of node.fields) { + field.fillPendingDefaults(context); + for (const [i, child] of field.children.entries()) { nexts.push([ { parent: p, diff --git a/packages/dds/tree/src/simple-tree/schemaTypes.ts b/packages/dds/tree/src/simple-tree/schemaTypes.ts index dd6e26102957..4f971495b971 100644 --- a/packages/dds/tree/src/simple-tree/schemaTypes.ts +++ b/packages/dds/tree/src/simple-tree/schemaTypes.ts @@ -8,7 +8,7 @@ import { Lazy } from "@fluidframework/core-utils/internal"; import { UsageError } from "@fluidframework/telemetry-utils/internal"; import type { FieldKey } from "../core/index.js"; -import type { NodeIdentifierManager } from "../feature-libraries/index.js"; +import type { FlexTreeHydratedContextMinimal } from "../feature-libraries/index.js"; import { type MakeNominal, brand, @@ -30,6 +30,7 @@ import type { TreeNode, TreeNodeSchemaCore, TreeNodeSchemaNonClass, + UnhydratedFlexTreeNode, } from "./core/index.js"; import { inPrototypeChain } from "./core/index.js"; import { isLazy, type FlexListToUnion, type LazyItem } from "./flexList.js"; @@ -296,17 +297,17 @@ export interface FieldProps { } /** - * A {@link FieldProvider} which requires additional context in order to produce its content + * A {@link FieldProvider} which prefers to have additional context in order to produce its content. */ export type ContextualFieldProvider = ( - context: NodeIdentifierManager, -) => InsertableContent | undefined; + context: FlexTreeHydratedContextMinimal | "UseGlobalContext", +) => UnhydratedFlexTreeNode | undefined; /** - * A {@link FieldProvider} which can produce its content in a vacuum + * A {@link FieldProvider} which can produce its content in a vacuum. */ -export type ConstantFieldProvider = () => InsertableContent | undefined; +export type ConstantFieldProvider = () => UnhydratedFlexTreeNode[]; /** - * A function which produces content for a field every time that it is called + * A function which produces content for a field every time that it is called. */ export type FieldProvider = ContextualFieldProvider | ConstantFieldProvider; /** diff --git a/packages/dds/tree/src/simple-tree/toMapTree.ts b/packages/dds/tree/src/simple-tree/toMapTree.ts index bfe2253ce68b..b1b5311dfd15 100644 --- a/packages/dds/tree/src/simple-tree/toMapTree.ts +++ b/packages/dds/tree/src/simple-tree/toMapTree.ts @@ -7,33 +7,21 @@ import { assert, fail, unreachableCase } from "@fluidframework/core-utils/intern import { UsageError } from "@fluidframework/telemetry-utils/internal"; import { isFluidHandle } from "@fluidframework/runtime-utils/internal"; -import { - EmptyKey, - type FieldKey, - type MapTree, - type TreeValue, - type ValueSchema, - type ExclusiveMapTree, -} from "../core/index.js"; -import { - isTreeValue, - valueSchemaAllows, - type NodeIdentifierManager, -} from "../feature-libraries/index.js"; -import { brand, isReadonlyArray, find, hasSome, hasSingle } from "../util/index.js"; +import { EmptyKey, type FieldKey, type TreeValue, type ValueSchema } from "../core/index.js"; +import { FieldKinds, isTreeValue, valueSchemaAllows } from "../feature-libraries/index.js"; +import { brand, isReadonlyArray, hasSome, hasSingle } from "../util/index.js"; import { nullSchema } from "./leafNodeSchema.js"; import { - type FieldSchema, type ImplicitAllowedTypes, normalizeAllowedTypes, - extractFieldProvider, isConstant, - type FieldProvider, type ImplicitFieldSchema, normalizeFieldSchema, FieldKind, type TreeLeafValue, + extractFieldProvider, + type ContextualFieldProvider, } from "./schemaTypes.js"; import { getKernel, @@ -45,6 +33,8 @@ import { type TreeNodeSchema, type Unhydrated, UnhydratedFlexTreeNode, + type Context, + UnhydratedTreeSequenceField, } from "./core/index.js"; // Required to prevent the introduction of new circular dependencies // TODO: Having the schema provide their own policy functions for compatibility which @@ -53,6 +43,10 @@ import { // eslint-disable-next-line import/no-internal-modules import { isObjectNodeSchema } from "./node-kinds/object/objectNodeTypes.js"; import type { IFluidHandle } from "@fluidframework/core-interfaces"; +import type { TreeNodeValid } from "./treeNodeValid.js"; +// eslint-disable-next-line import/no-internal-modules +import { createField, type UnhydratedFlexTreeField } from "./core/unhydratedFlexTree.js"; +import { convertFieldKind } from "./toStoredSchema.js"; /** * Module notes: @@ -93,8 +87,7 @@ import type { IFluidHandle } from "@fluidframework/core-interfaces"; export function mapTreeFromNodeData( data: TIn, allowedTypes: ImplicitFieldSchema, - context?: NodeIdentifierManager, -): TIn extends undefined ? undefined : ExclusiveMapTree { +): TIn extends undefined ? undefined : UnhydratedFlexTreeNode { const normalizedFieldSchema = normalizeFieldSchema(allowedTypes); if (data === undefined) { @@ -102,14 +95,15 @@ export function mapTreeFromNodeData( if (normalizedFieldSchema.kind !== FieldKind.Optional) { throw new UsageError("Got undefined for non-optional field."); } - return undefined as TIn extends undefined ? undefined : ExclusiveMapTree; + return undefined as TIn extends undefined ? undefined : UnhydratedFlexTreeNode; } - const mapTree = nodeDataToMapTree(data, normalizedFieldSchema.allowedTypeSet); - // Add what defaults can be provided. If no `context` is providing, some defaults may still be missing. - addDefaultsToMapTree(mapTree, normalizedFieldSchema.allowedTypes, context); + const mapTree: UnhydratedFlexTreeNode = nodeDataToMapTree( + data, + normalizedFieldSchema.allowedTypeSet, + ); - return mapTree as TIn extends undefined ? undefined : ExclusiveMapTree; + return mapTree as TIn extends undefined ? undefined : UnhydratedFlexTreeNode; } /** @@ -122,7 +116,7 @@ export function mapTreeFromNodeData( function nodeDataToMapTree( data: InsertableContent, allowedTypes: ReadonlySet, -): ExclusiveMapTree { +): UnhydratedFlexTreeNode { // A special cache path for processing unhydrated nodes. // They already have the mapTree, so there is no need to recompute it. const innerNode = tryGetInnerNode(data); @@ -131,12 +125,7 @@ function nodeDataToMapTree( if (!allowedTypes.has(getSimpleNodeSchemaFromInnerNode(innerNode))) { throw new UsageError("Invalid schema for this context."); } - // TODO: mapTreeFromNodeData modifies the trees it gets to add defaults. - // Using a cached value here can result in this tree having defaults applied to it more than once. - // This is unnecessary and inefficient, but should be a no-op if all calls provide the same context (which they might not). - // A cleaner design (avoiding this cast) might be to apply defaults eagerly if they don't need a context, and lazily (when hydrating) if they do. - // This could avoid having to mutate the map tree to apply defaults, removing the need for this cast. - return innerNode.mapTree; + return innerNode; } else { // The node is already hydrated, meaning that it already got inserted into the tree previously throw new UsageError("A node may not be inserted into the tree more than once"); @@ -147,7 +136,7 @@ function nodeDataToMapTree( const schema = getType(data, allowedTypes); - let result: ExclusiveMapTree; + let result: UnhydratedFlexTreeNode; switch (schema.kind) { case NodeKind.Leaf: result = leafToMapTree(data, schema, allowedTypes); @@ -179,7 +168,7 @@ function leafToMapTree( data: FactoryContent, schema: TreeNodeSchema, allowedTypes: ReadonlySet, -): ExclusiveMapTree { +): UnhydratedFlexTreeNode { assert(schema.kind === NodeKind.Leaf, 0x921 /* Expected a leaf schema. */); if (!isTreeValue(data)) { // This rule exists to protect against useless `toString` output like `[object Object]`. @@ -196,11 +185,20 @@ function leafToMapTree( 0x84a /* Unsupported schema for provided primitive. */, ); - return { - value: mappedValue, - type: brand(mappedSchema.identifier), - fields: new Map(), - }; + return new UnhydratedFlexTreeNode( + { + value: mappedValue, + type: brand(mappedSchema.identifier), + }, + new Map(), + contextFromSchema(schema), + ); +} + +function contextFromSchema(schema: TreeNodeSchema): Context { + const s = schema as typeof TreeNodeValid & TreeNodeSchema; + const context = s.oneTimeInitialize().oneTimeInitialized; + return context; } /** @@ -256,7 +254,7 @@ function mapValueWithFallbacks( function arrayChildToMapTree( child: InsertableContent, allowedTypes: ReadonlySet, -): ExclusiveMapTree { +): UnhydratedFlexTreeNode { // We do not support undefined sequence entries. // If we encounter an undefined entry, use null instead if supported by the schema, otherwise throw. let childWithFallback = child; @@ -278,7 +276,7 @@ function arrayChildToMapTree( * validation should happen. If it does, the input tree will be validated against this schema + policy, and an error will * be thrown if the tree does not conform to the schema. If undefined, no validation against the stored schema is done. */ -function arrayToMapTree(data: FactoryContent, schema: TreeNodeSchema): ExclusiveMapTree { +function arrayToMapTree(data: FactoryContent, schema: TreeNodeSchema): UnhydratedFlexTreeNode { assert(schema.kind === NodeKind.Array, 0x922 /* Expected an array schema. */); if (!(typeof data === "object" && data !== null && Symbol.iterator in data)) { throw new UsageError(`Input data is incompatible with Array schema: ${data}`); @@ -290,13 +288,31 @@ function arrayToMapTree(data: FactoryContent, schema: TreeNodeSchema): Exclusive arrayChildToMapTree(child, allowedChildTypes), ); - // Array nodes have a single `EmptyKey` field: - const fieldsEntries = mappedData.length === 0 ? [] : ([[EmptyKey, mappedData]] as const); + const context = contextFromSchema(schema); - return { - type: brand(schema.identifier), - fields: new Map(fieldsEntries), - }; + // Array nodes have a single `EmptyKey` field: + const fieldsEntries = + mappedData.length === 0 + ? [] + : ([ + [ + EmptyKey, + new UnhydratedTreeSequenceField( + context, + FieldKinds.sequence.identifier, + EmptyKey, + mappedData, + ), + ], + ] as const); + + return new UnhydratedFlexTreeNode( + { + type: brand(schema.identifier), + }, + new Map(fieldsEntries), + contextFromSchema(schema), + ); } /** @@ -307,7 +323,7 @@ function arrayToMapTree(data: FactoryContent, schema: TreeNodeSchema): Exclusive * validation should happen. If it does, the input tree will be validated against this schema + policy, and an error will * be thrown if the tree does not conform to the schema. If undefined, no validation against the stored schema is done. */ -function mapToMapTree(data: FactoryContent, schema: TreeNodeSchema): ExclusiveMapTree { +function mapToMapTree(data: FactoryContent, schema: TreeNodeSchema): UnhydratedFlexTreeNode { assert(schema.kind === NodeKind.Map, 0x923 /* Expected a Map schema. */); if (!(typeof data === "object" && data !== null)) { throw new UsageError(`Input data is incompatible with Map schema: ${data}`); @@ -323,7 +339,9 @@ function mapToMapTree(data: FactoryContent, schema: TreeNodeSchema): ExclusiveMa Object.entries(data) ) as Iterable; - const transformedFields = new Map(); + const context = contextFromSchema(schema); + + const transformedFields = new Map(); for (const item of fieldsIterator) { if (!isReadonlyArray(item) || item.length !== 2 || typeof item[0] !== "string") { throw new UsageError(`Input data is incompatible with map entry: ${item}`); @@ -333,15 +351,19 @@ function mapToMapTree(data: FactoryContent, schema: TreeNodeSchema): ExclusiveMa // Omit undefined values - an entry with an undefined value is equivalent to one that has been removed or omitted if (value !== undefined) { - const mappedField = nodeDataToMapTree(value, allowedChildTypes); - transformedFields.set(brand(key), [mappedField]); + const child = nodeDataToMapTree(value, allowedChildTypes); + const field = createField(context, FieldKinds.optional.identifier, brand(key), [child]); + transformedFields.set(brand(key), field); } } - return { - type: brand(schema.identifier), - fields: transformedFields, - }; + return new UnhydratedFlexTreeNode( + { + type: brand(schema.identifier), + }, + transformedFields, + context, + ); } /** @@ -349,7 +371,10 @@ function mapToMapTree(data: FactoryContent, schema: TreeNodeSchema): ExclusiveMa * @param data - The tree data to be transformed. Must be a Record-like object. * @param schema - The schema associated with the value. */ -function objectToMapTree(data: FactoryContent, schema: TreeNodeSchema): ExclusiveMapTree { +function objectToMapTree( + data: FactoryContent, + schema: TreeNodeSchema, +): UnhydratedFlexTreeNode { assert(isObjectNodeSchema(schema), 0x924 /* Expected an Object schema. */); if ( typeof data !== "object" || @@ -360,21 +385,36 @@ function objectToMapTree(data: FactoryContent, schema: TreeNodeSchema): Exclusiv throw new UsageError(`Input data is incompatible with Object schema: ${data}`); } - const fields = new Map(); + const fields = new Map(); + const context = contextFromSchema(schema); - // Loop through field keys without data. - // This does NOT apply defaults. for (const [key, fieldInfo] of schema.flexKeyMap) { - if (checkFieldProperty(data, key)) { - const value = (data as Record)[key as string]; - setFieldValue(fields, value, fieldInfo.schema, fieldInfo.storedKey); + const value = checkFieldProperty(data, key); + + let children: UnhydratedFlexTreeNode[] | ContextualFieldProvider; + if (value === undefined) { + const defaultProvider = + fieldInfo.schema.props?.defaultProvider ?? + fail("missing field has no default provider"); + const fieldProvider = extractFieldProvider(defaultProvider); + // eslint-disable-next-line unicorn/prefer-ternary + if (isConstant(fieldProvider)) { + children = fieldProvider(); + } else { + children = fieldProvider; + } + } else { + children = [nodeDataToMapTree(value, fieldInfo.schema.allowedTypeSet)]; } + + const kind = convertFieldKind.get(fieldInfo.schema.kind) ?? fail("Invalid field kind"); + fields.set( + fieldInfo.storedKey, + createField(context, kind.identifier, fieldInfo.storedKey, children), + ); } - return { - type: brand(schema.identifier), - fields, - }; + return new UnhydratedFlexTreeNode({ type: brand(schema.identifier) }, fields, context); } /** @@ -395,20 +435,6 @@ function checkFieldProperty( return Object.hasOwnProperty.call(data, key); } -function setFieldValue( - fields: Map, - fieldValue: InsertableContent | undefined, - fieldSchema: FieldSchema, - flexKey: FieldKey, -): void { - if (fieldValue !== undefined) { - const mappedChildTree = nodeDataToMapTree(fieldValue, fieldSchema.allowedTypeSet); - - assert(!fields.has(flexKey), 0x956 /* Keys must not be duplicated */); - fields.set(flexKey, [mappedChildTree]); - } -} - function getType( data: FactoryContent, allowedTypes: ReadonlySet, @@ -583,89 +609,6 @@ function allowsValue(schema: TreeNodeSchema, value: TreeValue): boolean { return false; } -/** - * Walk the given {@link ExclusiveMapTree} and deeply provide any field defaults for fields that are missing in the tree but present in the schema. - * @param mapTree - The tree to populate with defaults. This is borrowed: no references to it are kept by this function. - * @param allowedTypes - Some {@link TreeNodeSchema}, at least one of which the input tree must conform to - * @param context - An optional context for generating defaults. - * If present, all applicable defaults will be provided. - * If absent, only defaults produced by a {@link ConstantFieldProvider} will be provided, and defaults produced by a {@link ContextualFieldProvider} will be ignored. - * @remarks This function mutates the input tree by deeply adding new fields to the field maps where applicable. - */ -export function addDefaultsToMapTree( - mapTree: ExclusiveMapTree, - allowedTypes: ImplicitAllowedTypes, - context: NodeIdentifierManager | undefined, -): void { - const schema = - find(normalizeAllowedTypes(allowedTypes), (s) => s.identifier === mapTree.type) ?? - fail(0xae1 /* MapTree is incompatible with schema */); - - if (isObjectNodeSchema(schema)) { - for (const [_key, fieldInfo] of schema.flexKeyMap) { - const field = mapTree.fields.get(fieldInfo.storedKey); - if (field !== undefined) { - for (const child of field) { - addDefaultsToMapTree(child, fieldInfo.schema.allowedTypes, context); - } - } else { - const defaultProvider = fieldInfo.schema.props?.defaultProvider; - if (defaultProvider !== undefined) { - const fieldProvider = extractFieldProvider(defaultProvider); - const data = provideDefault(fieldProvider, context); - if (data !== undefined) { - setFieldValue(mapTree.fields, data, fieldInfo.schema, fieldInfo.storedKey); - // call addDefaultsToMapTree on newly inserted default values - for (const child of mapTree.fields.get(fieldInfo.storedKey) ?? - fail(0xae2 /* Expected field to be populated */)) { - addDefaultsToMapTree(child, fieldInfo.schema.allowedTypes, context); - } - } - } - } - } - return; - } - - switch (schema.kind) { - case NodeKind.Array: - case NodeKind.Map: - { - for (const field of mapTree.fields.values()) { - for (const child of field) { - addDefaultsToMapTree(child, schema.info as ImplicitAllowedTypes, context); - } - } - } - break; - default: - assert(schema.kind === NodeKind.Leaf, 0x989 /* Unrecognized schema kind */); - break; - } -} - -/** - * Provides the default value (which can be undefined, for example with optional fields), or undefined if a context is required but not provided. - * @privateRemarks - * It is a bit concerning that there is no way for the caller to know when undefined is returned if that is the default value, or a context was required. - * TODO: maybe better formalize the two stage defaulting (without then with context), or rework this design we only do one stage. - */ -function provideDefault( - fieldProvider: FieldProvider, - context: NodeIdentifierManager | undefined, -): InsertableContent | undefined { - if (context !== undefined) { - return fieldProvider(context); - } else { - if (isConstant(fieldProvider)) { - return fieldProvider(); - } else { - // Leaving field empty despite it needing a default value since a context was required and none was provided. - // Caller better handle this case by providing the default at some other point in time when the context becomes known. - } - } -} - /** * Retrieves the InnerNode associated with the given target via {@link setInnerNode}, if any. * @remarks diff --git a/packages/dds/tree/src/simple-tree/toStoredSchema.ts b/packages/dds/tree/src/simple-tree/toStoredSchema.ts index d6bc3ac35531..b42bd1af236c 100644 --- a/packages/dds/tree/src/simple-tree/toStoredSchema.ts +++ b/packages/dds/tree/src/simple-tree/toStoredSchema.ts @@ -93,7 +93,10 @@ export function convertField(schema: SimpleFieldSchema): TreeFieldStoredSchema { return { kind, types }; } -const convertFieldKind = new Map([ +export const convertFieldKind: ReadonlyMap = new Map< + FieldKind, + FlexFieldKind +>([ [FieldKind.Optional, FieldKinds.optional], [FieldKind.Required, FieldKinds.required], [FieldKind.Identifier, FieldKinds.identifier], diff --git a/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts b/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts index cf19711c32f8..317c40faa903 100644 --- a/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts +++ b/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts @@ -131,7 +131,7 @@ describe("SchematizingSimpleTreeView", () => { // so this hack using internal APIs is needed a workaround to test the additional schema validation layer. // In production cases this extra validation exists to help prevent corruption when bugs // allow invalid data through the public API. - (child.mapTree as Mutable).value = "invalid value"; + (child.data as Mutable).value = "invalid value"; // Attempt to initialize with invalid content if (enableSchemaValidation || additionalAsserts) { diff --git a/packages/dds/tree/src/test/simple-tree/treeNodeValid.spec.ts b/packages/dds/tree/src/test/simple-tree/treeNodeValid.spec.ts index 3671a572866b..2d2c5c3541ee 100644 --- a/packages/dds/tree/src/test/simple-tree/treeNodeValid.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/treeNodeValid.spec.ts @@ -35,9 +35,9 @@ describe("TreeNodeValid", () => { class MockFlexNode extends UnhydratedFlexTreeNode { public constructor(public readonly simpleSchema: TreeNodeSchema) { super( + { type: brand(simpleSchema.identifier) }, + new Map(), getUnhydratedContext(simpleSchema), - { fields: new Map(), type: brand(simpleSchema.identifier) }, - undefined, ); } } From 3d031e2e08569bedd207cf31c116ba3d99f45d65 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Wed, 28 May 2025 09:57:43 -0700 Subject: [PATCH 02/25] more fixes --- .../default-schema/defaultEditBuilder.ts | 4 +- .../simple-tree/core/unhydratedFlexTree.ts | 22 +++------- .../dds/tree/src/simple-tree/toMapTree.ts | 44 ++++++++++--------- 3 files changed, 32 insertions(+), 38 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts index 04e21e41bdea..e208f7bc296e 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts @@ -435,7 +435,7 @@ export interface OptionalFieldEditBuilder { /** */ -export interface SequenceFieldEditBuilder { +export interface SequenceFieldEditBuilder { /** * Issues a change which inserts the `newContent` at the given `index`. * @param index - the index at which to insert the `newContent`. @@ -448,7 +448,7 @@ export interface SequenceFieldEditBuilder { * @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; } /** diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index f49df3f9a6d6..8752ff250385 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -45,6 +45,7 @@ import { type FlexibleNodeContent, type FlexTreeHydratedContextMinimal, cursorForMapTreeNode, + type FlexibleFieldContent, } from "../../feature-libraries/index.js"; import { brand, getOrCreate } from "../../util/index.js"; @@ -57,15 +58,7 @@ import type { } from "../../feature-libraries/mapTreeCursor.js"; interface UnhydratedTreeSequenceFieldEditBuilder - extends SequenceFieldEditBuilder { - /** - * Issues a change which removes `count` elements starting at the given `index`. - * @param index - The index of the first removed element. - * @param count - The number of elements to remove. - * @returns the MapTrees that were removed - */ - remove(index: number, count: number): UnhydratedFlexTreeNode[]; -} + extends SequenceFieldEditBuilder {} type UnhydratedFlexTreeNodeEvents = Pick; @@ -86,6 +79,8 @@ interface LocationInField { export class UnhydratedFlexTreeNode implements FlexTreeNode, MapTreeNodeViewGeneric { + private location = unparentedLocation; + public get storedSchema(): TreeNodeStoredSchema { return ( this.context.schema.nodeSchema.get(this.data.type) ?? fail(0xb46 /* missing schema */) @@ -115,7 +110,6 @@ export class UnhydratedFlexTreeNode public readonly data: NodeData, public readonly fields: Map, public readonly simpleContext: Context, - private location = unparentedLocation, ) { for (const [_key, field] of this.fields) { field.parent = this; @@ -432,7 +426,7 @@ export class EagerMapTreeOptionalField } } -export class EagerMapTreeRequiredField +class EagerMapTreeRequiredField extends EagerMapTreeOptionalField implements FlexTreeRequiredField { @@ -450,7 +444,7 @@ export class UnhydratedTreeSequenceField extends UnhydratedFlexTreeField implements FlexTreeSequenceField { - public readonly editor: UnhydratedTreeSequenceFieldEditBuilder = { + public readonly editor = { insert: (index, newContent): void => { for (let i = 0; i < newContent.length; i++) { const c = newContent[i]; @@ -480,7 +474,7 @@ export class UnhydratedTreeSequenceField }); return removed ?? fail(0xb4a /* Expected removed to be set by edit */); }, - }; + } satisfies UnhydratedTreeSequenceFieldEditBuilder; public at(index: number): FlexTreeUnknownUnboxed | undefined { const i = indexForAt(index, this.length); @@ -509,10 +503,8 @@ export function createField( case FieldKinds.required.identifier: case FieldKinds.identifier.identifier: return new EagerMapTreeRequiredField(...args); - case FieldKinds.optional.identifier: return new EagerMapTreeOptionalField(...args); - case FieldKinds.sequence.identifier: return new UnhydratedTreeSequenceField(...args); case FieldKinds.forbidden.identifier: diff --git a/packages/dds/tree/src/simple-tree/toMapTree.ts b/packages/dds/tree/src/simple-tree/toMapTree.ts index b1b5311dfd15..3c43a8dc136a 100644 --- a/packages/dds/tree/src/simple-tree/toMapTree.ts +++ b/packages/dds/tree/src/simple-tree/toMapTree.ts @@ -7,7 +7,13 @@ import { assert, fail, unreachableCase } from "@fluidframework/core-utils/intern import { UsageError } from "@fluidframework/telemetry-utils/internal"; import { isFluidHandle } from "@fluidframework/runtime-utils/internal"; -import { EmptyKey, type FieldKey, type TreeValue, type ValueSchema } from "../core/index.js"; +import { + EmptyKey, + type FieldKey, + type NodeData, + type TreeValue, + type ValueSchema, +} from "../core/index.js"; import { FieldKinds, isTreeValue, valueSchemaAllows } from "../feature-libraries/index.js"; import { brand, isReadonlyArray, hasSome, hasSingle } from "../util/index.js"; @@ -136,7 +142,7 @@ function nodeDataToMapTree( const schema = getType(data, allowedTypes); - let result: UnhydratedFlexTreeNode; + let result: FlexContent; switch (schema.kind) { case NodeKind.Leaf: result = leafToMapTree(data, schema, allowedTypes); @@ -154,9 +160,11 @@ function nodeDataToMapTree( unreachableCase(schema.kind); } - return result; + return new UnhydratedFlexTreeNode(...result, contextFromSchema(schema)); } +type FlexContent = [NodeData, Map]; + /** * Transforms data under a Leaf schema. * @param data - The tree data to be transformed. Must be a {@link TreeValue}. @@ -168,7 +176,7 @@ function leafToMapTree( data: FactoryContent, schema: TreeNodeSchema, allowedTypes: ReadonlySet, -): UnhydratedFlexTreeNode { +): FlexContent { assert(schema.kind === NodeKind.Leaf, 0x921 /* Expected a leaf schema. */); if (!isTreeValue(data)) { // This rule exists to protect against useless `toString` output like `[object Object]`. @@ -185,14 +193,13 @@ function leafToMapTree( 0x84a /* Unsupported schema for provided primitive. */, ); - return new UnhydratedFlexTreeNode( + return [ { value: mappedValue, type: brand(mappedSchema.identifier), }, - new Map(), - contextFromSchema(schema), - ); + new Map(), + ]; } function contextFromSchema(schema: TreeNodeSchema): Context { @@ -276,7 +283,7 @@ function arrayChildToMapTree( * validation should happen. If it does, the input tree will be validated against this schema + policy, and an error will * be thrown if the tree does not conform to the schema. If undefined, no validation against the stored schema is done. */ -function arrayToMapTree(data: FactoryContent, schema: TreeNodeSchema): UnhydratedFlexTreeNode { +function arrayToMapTree(data: FactoryContent, schema: TreeNodeSchema): FlexContent { assert(schema.kind === NodeKind.Array, 0x922 /* Expected an array schema. */); if (!(typeof data === "object" && data !== null && Symbol.iterator in data)) { throw new UsageError(`Input data is incompatible with Array schema: ${data}`); @@ -306,13 +313,12 @@ function arrayToMapTree(data: FactoryContent, schema: TreeNodeSchema): Unhydrate ], ] as const); - return new UnhydratedFlexTreeNode( + return [ { type: brand(schema.identifier), }, new Map(fieldsEntries), - contextFromSchema(schema), - ); + ]; } /** @@ -323,7 +329,7 @@ function arrayToMapTree(data: FactoryContent, schema: TreeNodeSchema): Unhydrate * validation should happen. If it does, the input tree will be validated against this schema + policy, and an error will * be thrown if the tree does not conform to the schema. If undefined, no validation against the stored schema is done. */ -function mapToMapTree(data: FactoryContent, schema: TreeNodeSchema): UnhydratedFlexTreeNode { +function mapToMapTree(data: FactoryContent, schema: TreeNodeSchema): FlexContent { assert(schema.kind === NodeKind.Map, 0x923 /* Expected a Map schema. */); if (!(typeof data === "object" && data !== null)) { throw new UsageError(`Input data is incompatible with Map schema: ${data}`); @@ -357,13 +363,12 @@ function mapToMapTree(data: FactoryContent, schema: TreeNodeSchema): UnhydratedF } } - return new UnhydratedFlexTreeNode( + return [ { type: brand(schema.identifier), }, transformedFields, - context, - ); + ]; } /** @@ -371,10 +376,7 @@ function mapToMapTree(data: FactoryContent, schema: TreeNodeSchema): UnhydratedF * @param data - The tree data to be transformed. Must be a Record-like object. * @param schema - The schema associated with the value. */ -function objectToMapTree( - data: FactoryContent, - schema: TreeNodeSchema, -): UnhydratedFlexTreeNode { +function objectToMapTree(data: FactoryContent, schema: TreeNodeSchema): FlexContent { assert(isObjectNodeSchema(schema), 0x924 /* Expected an Object schema. */); if ( typeof data !== "object" || @@ -414,7 +416,7 @@ function objectToMapTree( ); } - return new UnhydratedFlexTreeNode({ type: brand(schema.identifier) }, fields, context); + return [{ type: brand(schema.identifier) }, fields]; } /** From 8a6c2a0b159fc6078ea1e435d5cf624d041e996e Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Wed, 28 May 2025 12:34:03 -0700 Subject: [PATCH 03/25] Fix build --- .../dds/tree/src/shared-tree/treeAlpha.ts | 4 +- .../dds/tree/src/simple-tree/api/create.ts | 61 ++++++++- .../tree/src/simple-tree/api/schemaFactory.ts | 8 +- .../dds/tree/src/simple-tree/core/index.ts | 2 +- .../simple-tree/core/unhydratedFlexTree.ts | 34 +++-- .../simple-tree/node-kinds/array/arrayNode.ts | 4 +- .../dds/tree/src/simple-tree/schemaTypes.ts | 2 +- .../dds/tree/src/simple-tree/toMapTree.ts | 21 +-- .../shared-tree/schematizingTreeView.spec.ts | 2 - .../core/unhydratedFlexTree.spec.ts | 84 ++++++------ .../src/test/simple-tree/toMapTree.spec.ts | 126 +++++++++++++----- .../test/simple-tree/unhydratedNode.spec.ts | 21 ++- packages/dds/tree/src/test/utils.ts | 7 +- 13 files changed, 244 insertions(+), 132 deletions(-) diff --git a/packages/dds/tree/src/shared-tree/treeAlpha.ts b/packages/dds/tree/src/shared-tree/treeAlpha.ts index dd17e5973307..63cf55855a0c 100644 --- a/packages/dds/tree/src/shared-tree/treeAlpha.ts +++ b/packages/dds/tree/src/shared-tree/treeAlpha.ts @@ -59,7 +59,7 @@ import { independentInitializedView, type ViewContent } from "./independentView. import { SchematizingSimpleTreeView, ViewSlot } from "./schematizingTreeView.js"; import { currentVersion } from "../codec/index.js"; // eslint-disable-next-line import/no-internal-modules -import { createTreeNodeFromInner } from "../simple-tree/core/treeNodeKernel.js"; +import { getOrCreateNodeFromInnerNode } from "../simple-tree/core/index.js"; const identifier: TreeIdentifierUtils = (node: TreeNode): string | undefined => { const nodeIdentifier = getIdentifierFromNode(node, "uncompressed"); @@ -369,7 +369,7 @@ export const TreeAlpha: TreeAlpha = { : TreeNode | TreeLeafValue | undefined > { const mapTree = mapTreeFromNodeData(data as InsertableField, schema); - const result = mapTree === undefined ? undefined : createTreeNodeFromInner(mapTree); + const result = mapTree === undefined ? undefined : getOrCreateNodeFromInnerNode(mapTree); return result as Unhydrated< TSchema extends ImplicitFieldSchema ? TreeFieldFromImplicitField diff --git a/packages/dds/tree/src/simple-tree/api/create.ts b/packages/dds/tree/src/simple-tree/api/create.ts index c27ab7ff1cf9..98f4b9b75589 100644 --- a/packages/dds/tree/src/simple-tree/api/create.ts +++ b/packages/dds/tree/src/simple-tree/api/create.ts @@ -5,27 +5,46 @@ import { assert, fail } from "@fluidframework/core-utils/internal"; -import type { ITreeCursorSynchronous, SchemaAndPolicy } from "../../core/index.js"; +import { + CursorLocationType, + mapCursorField, + mapCursorFields, + type ITreeCursorSynchronous, + type SchemaAndPolicy, +} from "../../core/index.js"; import type { ImplicitFieldSchema, TreeFieldFromImplicitField } from "../schemaTypes.js"; -import type { Unhydrated } from "../core/index.js"; +import { + type Context, + getOrCreateNodeFromInnerNode, + type NodeKind, + type Unhydrated, + UnhydratedFlexTreeNode, +} from "../core/index.js"; import { defaultSchemaPolicy, inSchemaOrThrow, - mapTreeFromCursor, isFieldInSchema, } from "../../feature-libraries/index.js"; import { getUnhydratedContext } from "../createContext.js"; import { createUnknownOptionalFieldPolicy } from "../node-kinds/index.js"; +import type { SimpleNodeSchema, SimpleNodeSchemaBase } from "../simpleSchema.js"; +// eslint-disable-next-line import/no-internal-modules +import { createField } from "../core/unhydratedFlexTree.js"; +import { getStoredSchema } from "../toStoredSchema.js"; /** * Creates an unhydrated simple-tree field from a cursor in nodes mode. + * @remarks + * Does not support defaults. + * Validates the field is in schema. */ export function createFromCursor( schema: TSchema, cursor: ITreeCursorSynchronous | undefined, ): Unhydrated> { - const mapTrees = cursor === undefined ? [] : [mapTreeFromCursor(cursor)]; const context = getUnhydratedContext(schema); + const mapTrees = cursor === undefined ? [] : [flexTreeFromCursor(context, cursor)]; + const flexSchema = context.flexContext.schema; const schemaValidationPolicy: SchemaAndPolicy = { @@ -49,7 +68,37 @@ export function createFromCursor( assert(mapTrees.length === 1, 0xa11 /* unexpected field length */); // Length asserted above, so this is safe. This assert is done instead of checking for undefined after indexing to ensure a length greater than 1 also errors. // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const _mapTree = mapTrees[0]!; + const mapTree = mapTrees[0]!; + + return getOrCreateNodeFromInnerNode(mapTree) as Unhydrated< + TreeFieldFromImplicitField + >; +} - return fail("TODO"); +/** + * Construct an {@link UnhydratedFlexTreeNode} from a cursor in Nodes mode. + * @remarks + * This does not validate the node is in schema. + */ +export function flexTreeFromCursor( + context: Context, + cursor: ITreeCursorSynchronous, +): UnhydratedFlexTreeNode { + assert(cursor.mode === CursorLocationType.Nodes, "Expected nodes cursor"); + const schema = context.schema.get(cursor.type) ?? fail("missing schema"); + const storedSchema = getStoredSchema( + schema as SimpleNodeSchemaBase as SimpleNodeSchema, + ); + const fields = new Map( + mapCursorFields(cursor, () => [ + cursor.getFieldKey(), + createField( + context.flexContext, + storedSchema.getFieldSchema(cursor.getFieldKey()).kind, + cursor.getFieldKey(), + mapCursorField(cursor, () => flexTreeFromCursor(context, cursor)), + ), + ]), + ); + return new UnhydratedFlexTreeNode({ type: cursor.type }, fields, context); } diff --git a/packages/dds/tree/src/simple-tree/api/schemaFactory.ts b/packages/dds/tree/src/simple-tree/api/schemaFactory.ts index 27f922800f7a..9cf1950827b1 100644 --- a/packages/dds/tree/src/simple-tree/api/schemaFactory.ts +++ b/packages/dds/tree/src/simple-tree/api/schemaFactory.ts @@ -300,9 +300,7 @@ export interface SchemaStatics { ) => System_Unsafe.FieldSchemaUnsafe; } -const defaultOptionalProvider: DefaultProvider = getDefaultProvider(() => { - return undefined; -}); +const defaultOptionalProvider: DefaultProvider = getDefaultProvider(() => []); // The following overloads for optional and required are used to get around the fact that // the compiler can't infer that UnannotateImplicitAllowedTypes is equal to T when T is known to extend ImplicitAllowedTypes @@ -1126,7 +1124,7 @@ export class SchemaFactory< const defaultIdentifierProvider: DefaultProvider = getDefaultProvider( ( context: FlexTreeHydratedContextMinimal | "UseGlobalContext", - ): UnhydratedFlexTreeNode => { + ): UnhydratedFlexTreeNode[] => { const id = context === "UseGlobalContext" ? globalIdentifierAllocator.decompress( @@ -1136,7 +1134,7 @@ export class SchemaFactory< context.nodeKeyManager.generateLocalNodeIdentifier(), ); - return mapTreeFromNodeData(id, this.string); + return [mapTreeFromNodeData(id, this.string)]; }, ); return createFieldSchema(FieldKind.Identifier, this.string, { diff --git a/packages/dds/tree/src/simple-tree/core/index.ts b/packages/dds/tree/src/simple-tree/core/index.ts index c150e4ab9173..6530755184a4 100644 --- a/packages/dds/tree/src/simple-tree/core/index.ts +++ b/packages/dds/tree/src/simple-tree/core/index.ts @@ -38,6 +38,6 @@ export { Context, HydratedContext, SimpleContextSlot } from "./context.js"; export { getOrCreateNodeFromInnerNode } from "./getOrCreateNode.js"; export { UnhydratedFlexTreeNode, - UnhydratedTreeSequenceField, + UnhydratedSequenceField, UnhydratedContext, } from "./unhydratedFlexTree.js"; diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index 14cf70e95336..66d1c4be69fa 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -127,7 +127,7 @@ export class UnhydratedFlexTreeNode private getOrCreateField(key: FieldKey): UnhydratedFlexTreeField { return getOrCreate(this.fields, key, () => { const stored = this.storedSchema.getFieldSchema(key).kind; - const field = createField(this.simpleContext, stored, key, []); + const field = createField(this.context, stored, key, []); field.parent = this; return field; }); @@ -289,14 +289,10 @@ export class UnhydratedFlexTreeField { public [flexTreeMarker] = FlexTreeEntityKind.Field as const; - public get context(): FlexTreeContext { - return this.simpleContext.flexContext; - } - public parent: UnhydratedFlexTreeNode | undefined = undefined; public constructor( - public readonly simpleContext: Context, + public readonly context: FlexTreeContext, public readonly schema: FieldKindIdentifier, public readonly key: FieldKey, private lazyChildren: UnhydratedFlexTreeNode[] | ContextualFieldProvider, @@ -323,7 +319,7 @@ export class UnhydratedFlexTreeField const provider = this.getPendingDefault(); if (provider) { const content = provider(context); - this.lazyChildren = content === undefined ? [] : [content]; + this.lazyChildren = content; } } @@ -335,7 +331,7 @@ export class UnhydratedFlexTreeField const provider = this.getPendingDefault(); if (provider) { const content = provider("UseGlobalContext"); - this.lazyChildren = content === undefined ? [] : [content]; + this.lazyChildren = content; } return this.lazyChildren as UnhydratedFlexTreeNode[]; } @@ -402,7 +398,7 @@ export class UnhydratedFlexTreeField } } -export class EagerMapTreeOptionalField +export class UnhydratedOptionalField extends UnhydratedFlexTreeField implements FlexTreeOptionalField { @@ -433,8 +429,8 @@ export class EagerMapTreeOptionalField } } -class EagerMapTreeRequiredField - extends EagerMapTreeOptionalField +class UnhydratedRequiredField + extends UnhydratedOptionalField implements FlexTreeRequiredField { public override get content(): FlexTreeUnknownUnboxed { @@ -447,7 +443,7 @@ class EagerMapTreeRequiredField } } -export class UnhydratedTreeSequenceField +export class UnhydratedSequenceField extends UnhydratedFlexTreeField implements FlexTreeSequenceField { @@ -455,14 +451,16 @@ export class UnhydratedTreeSequenceField insert: (index, newContent): void => { for (const c of newContent) { assert(c !== undefined, 0xa0a /* Unexpected sparse array content */); + assert(c instanceof UnhydratedFlexTreeNode, "Expected unhydrated node"); } + const newContentChecked = newContent as readonly UnhydratedFlexTreeNode[]; this.edit((mapTrees) => { if (newContent.length < 1000) { // For "smallish arrays" (`1000` is not empirically derived), the `splice` function is appropriate... - mapTrees.splice(index, 0, ...newContent); + mapTrees.splice(index, 0, ...newContentChecked); } else { // ...but we avoid using `splice` + spread for very large input arrays since there is a limit on how many elements can be spread (too many will overflow the stack). - return mapTrees.slice(0, index).concat(newContent, mapTrees.slice(index)); + return mapTrees.slice(0, index).concat(newContentChecked, mapTrees.slice(index)); } }); }, @@ -496,7 +494,7 @@ export class UnhydratedTreeSequenceField /** Creates a field with the given attributes */ export function createField( ...args: [ - Context, + FlexTreeContext, FieldKindIdentifier, FieldKey, UnhydratedFlexTreeNode[] | ContextualFieldProvider, @@ -505,11 +503,11 @@ export function createField( switch (args[1]) { case FieldKinds.required.identifier: case FieldKinds.identifier.identifier: - return new EagerMapTreeRequiredField(...args); + return new UnhydratedRequiredField(...args); case FieldKinds.optional.identifier: - return new EagerMapTreeOptionalField(...args); + return new UnhydratedOptionalField(...args); case FieldKinds.sequence.identifier: - return new UnhydratedTreeSequenceField(...args); + return new UnhydratedSequenceField(...args); case FieldKinds.forbidden.identifier: // TODO: this seems to used by unknown optional fields. They should probably use "optional" not "Forbidden" schema. return new UnhydratedFlexTreeField(...args); diff --git a/packages/dds/tree/src/simple-tree/node-kinds/array/arrayNode.ts b/packages/dds/tree/src/simple-tree/node-kinds/array/arrayNode.ts index 67a07b9beec0..b642662dade6 100644 --- a/packages/dds/tree/src/simple-tree/node-kinds/array/arrayNode.ts +++ b/packages/dds/tree/src/simple-tree/node-kinds/array/arrayNode.ts @@ -40,7 +40,7 @@ import { type TreeNodeSchemaClass, getKernel, type UnhydratedFlexTreeNode, - UnhydratedTreeSequenceField, + UnhydratedSequenceField, } from "../../core/index.js"; import { type InsertableContent, mapTreeFromNodeData } from "../../toMapTree.js"; import { prepareArrayContentForInsertion } from "../../prepareForInsertion.js"; @@ -1013,7 +1013,7 @@ abstract class CustomArrayNodeBase const movedCount = sourceEnd - sourceStart; if (!destinationField.context.isHydrated()) { - if (!(sourceField instanceof UnhydratedTreeSequenceField)) { + if (!(sourceField instanceof UnhydratedSequenceField)) { throw new UsageError( "Cannot move elements from a hydrated array to an unhydrated array.", ); diff --git a/packages/dds/tree/src/simple-tree/schemaTypes.ts b/packages/dds/tree/src/simple-tree/schemaTypes.ts index 4f971495b971..16c68960cd66 100644 --- a/packages/dds/tree/src/simple-tree/schemaTypes.ts +++ b/packages/dds/tree/src/simple-tree/schemaTypes.ts @@ -301,7 +301,7 @@ export interface FieldProps { */ export type ContextualFieldProvider = ( context: FlexTreeHydratedContextMinimal | "UseGlobalContext", -) => UnhydratedFlexTreeNode | undefined; +) => UnhydratedFlexTreeNode[]; /** * A {@link FieldProvider} which can produce its content in a vacuum. */ diff --git a/packages/dds/tree/src/simple-tree/toMapTree.ts b/packages/dds/tree/src/simple-tree/toMapTree.ts index 3c43a8dc136a..c2189f0e90d4 100644 --- a/packages/dds/tree/src/simple-tree/toMapTree.ts +++ b/packages/dds/tree/src/simple-tree/toMapTree.ts @@ -39,8 +39,7 @@ import { type TreeNodeSchema, type Unhydrated, UnhydratedFlexTreeNode, - type Context, - UnhydratedTreeSequenceField, + UnhydratedSequenceField, } from "./core/index.js"; // Required to prevent the introduction of new circular dependencies // TODO: Having the schema provide their own policy functions for compatibility which @@ -49,10 +48,10 @@ import { // eslint-disable-next-line import/no-internal-modules import { isObjectNodeSchema } from "./node-kinds/object/objectNodeTypes.js"; import type { IFluidHandle } from "@fluidframework/core-interfaces"; -import type { TreeNodeValid } from "./treeNodeValid.js"; // eslint-disable-next-line import/no-internal-modules import { createField, type UnhydratedFlexTreeField } from "./core/unhydratedFlexTree.js"; import { convertFieldKind } from "./toStoredSchema.js"; +import { getUnhydratedContext } from "./createContext.js"; /** * Module notes: @@ -160,7 +159,7 @@ function nodeDataToMapTree( unreachableCase(schema.kind); } - return new UnhydratedFlexTreeNode(...result, contextFromSchema(schema)); + return new UnhydratedFlexTreeNode(...result, getUnhydratedContext(schema)); } type FlexContent = [NodeData, Map]; @@ -202,12 +201,6 @@ function leafToMapTree( ]; } -function contextFromSchema(schema: TreeNodeSchema): Context { - const s = schema as typeof TreeNodeValid & TreeNodeSchema; - const context = s.oneTimeInitialize().oneTimeInitialized; - return context; -} - /** * Checks an incoming {@link TreeLeafValue} to ensure it is compatible with its requirements. * For unsupported values with a schema-compatible replacement, return the replacement value. @@ -295,7 +288,7 @@ function arrayToMapTree(data: FactoryContent, schema: TreeNodeSchema): FlexConte arrayChildToMapTree(child, allowedChildTypes), ); - const context = contextFromSchema(schema); + const context = getUnhydratedContext(schema).flexContext; // Array nodes have a single `EmptyKey` field: const fieldsEntries = @@ -304,7 +297,7 @@ function arrayToMapTree(data: FactoryContent, schema: TreeNodeSchema): FlexConte : ([ [ EmptyKey, - new UnhydratedTreeSequenceField( + new UnhydratedSequenceField( context, FieldKinds.sequence.identifier, EmptyKey, @@ -345,7 +338,7 @@ function mapToMapTree(data: FactoryContent, schema: TreeNodeSchema): FlexContent Object.entries(data) ) as Iterable; - const context = contextFromSchema(schema); + const context = getUnhydratedContext(schema).flexContext; const transformedFields = new Map(); for (const item of fieldsIterator) { @@ -388,7 +381,7 @@ function objectToMapTree(data: FactoryContent, schema: TreeNodeSchema): FlexCont } const fields = new Map(); - const context = contextFromSchema(schema); + const context = getUnhydratedContext(schema).flexContext; for (const [key, fieldInfo] of schema.flexKeyMap) { const value = checkFieldProperty(data, key); diff --git a/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts b/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts index 317c40faa903..4983fccda241 100644 --- a/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts +++ b/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts @@ -58,12 +58,10 @@ const configGeneralized2 = new TreeViewConfiguration({ function checkoutWithInitialTree( viewConfig: TreeViewConfiguration, unhydratedInitialTree: InsertableField, - nodeKeyManager = new MockNodeIdentifierManager(), ): TreeCheckout { const initialTree = fieldCursorFromInsertable( viewConfig.schema, unhydratedInitialTree, - nodeKeyManager, ); const treeContent: TreeStoredContent = { schema: toStoredSchema(viewConfig.schema), diff --git a/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts b/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts index 06d69a77c751..0ba7ad4da8e5 100644 --- a/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts @@ -5,21 +5,24 @@ import { strict as assert } from "node:assert"; -import { FieldKinds, type FlexTreeOptionalField } from "../../../feature-libraries/index.js"; +import { cursorForMapTreeNode, FieldKinds } from "../../../feature-libraries/index.js"; import { deepCopyMapTree, EmptyKey, type ExclusiveMapTree, type FieldKey, + type Value, } from "../../../core/index.js"; import { brand } from "../../../util/index.js"; -import { - UnhydratedFlexTreeNode, +import type { + UnhydratedOptionalField, // eslint-disable-next-line import/no-internal-modules } from "../../../simple-tree/core/unhydratedFlexTree.js"; import { SchemaFactory, stringSchema } from "../../../simple-tree/index.js"; // eslint-disable-next-line import/no-internal-modules import { getUnhydratedContext } from "../../../simple-tree/createContext.js"; +// eslint-disable-next-line import/no-internal-modules +import { flexTreeFromCursor } from "../../../simple-tree/api/create.js"; describe("unhydratedFlexTree", () => { // #region The schema used in this test suite @@ -72,15 +75,9 @@ describe("unhydratedFlexTree", () => { arrayNodeSchemaSimple, objectSchemaSimple, ]); - const map = UnhydratedFlexTreeNode.getOrCreate(context, mapMapTree); - const arrayNode = UnhydratedFlexTreeNode.getOrCreate(context, fieldNodeMapTree); - const object = UnhydratedFlexTreeNode.getOrCreate(context, objectMapTree); - - it("are cached", () => { - assert.equal(UnhydratedFlexTreeNode.getOrCreate(context, mapMapTree), map); - assert.equal(UnhydratedFlexTreeNode.getOrCreate(context, fieldNodeMapTree), arrayNode); - assert.equal(UnhydratedFlexTreeNode.getOrCreate(context, objectMapTree), object); - }); + const map = flexTreeFromCursor(context, cursorForMapTreeNode(mapMapTree)); + const arrayNode = flexTreeFromCursor(context, cursorForMapTreeNode(fieldNodeMapTree)); + const object = flexTreeFromCursor(context, cursorForMapTreeNode(objectMapTree)); it("can get their type", () => { assert.equal(map.type, "Test.Map"); @@ -149,10 +146,13 @@ describe("unhydratedFlexTree", () => { it("cannot be multiparented", () => { assert.throws(() => - UnhydratedFlexTreeNode.getOrCreate(context, { - type: brand("Parent of a node that already has another parent"), - fields: new Map([[brand("fieldKey"), [mapMapTree]]]), - }), + flexTreeFromCursor( + context, + cursorForMapTreeNode({ + type: brand("Parent of a node that already has another parent"), + fields: new Map([[brand("fieldKey"), [mapMapTree]]]), + }), + ), ); const duplicateChild: ExclusiveMapTree = { @@ -161,10 +161,13 @@ describe("unhydratedFlexTree", () => { fields: new Map(), }; assert.throws(() => { - UnhydratedFlexTreeNode.getOrCreate(context, { - type: brand("Parent with the same child twice in the same field"), - fields: new Map([[EmptyKey, [duplicateChild, duplicateChild]]]), - }); + flexTreeFromCursor( + context, + cursorForMapTreeNode({ + type: brand("Parent with the same child twice in the same field"), + fields: new Map([[EmptyKey, [duplicateChild, duplicateChild]]]), + }), + ); }); }); @@ -199,50 +202,50 @@ describe("unhydratedFlexTree", () => { const mutableObjectMapTree = deepCopyMapTree(objectMapTree); const mutableObjectMapTreeMap = mutableObjectMapTree.fields.get(objectMapKey)?.[0]; assert(mutableObjectMapTreeMap !== undefined); - const mutableObject = UnhydratedFlexTreeNode.getOrCreate(context, mutableObjectMapTree); - const field = mutableObject.getBoxed(objectMapKey) as FlexTreeOptionalField; + const mutableObject = flexTreeFromCursor( + context, + cursorForMapTreeNode(mutableObjectMapTree), + ); + const field = mutableObject.getBoxed(objectMapKey) as UnhydratedOptionalField; const oldMap = field.boxedAt(0); assert(oldMap !== undefined); assert.equal(oldMap.parentField.parent.parent, mutableObject); - const newMap = UnhydratedFlexTreeNode.getOrCreate(context, deepCopyMapTree(mapMapTree)); + const newMap = flexTreeFromCursor(context, cursorForMapTreeNode(mapMapTree)); assert.notEqual(newMap, oldMap); assert.equal(newMap.parentField.parent.parent, undefined); // Replace the old map with a new map - field.editor.set(newMap.mapTree, false); + field.editor.set(newMap); assert.equal(oldMap.parentField.parent.parent, undefined); assert.equal(newMap.parentField.parent.parent, mutableObject); assert.equal(field.boxedAt(0), newMap); // Replace the new map with the old map again - field.editor.set(mutableObjectMapTreeMap, false); + field.editor.set(mutableObjectMapTreeMap); assert.equal(oldMap.parentField.parent.parent, mutableObject); assert.equal(newMap.parentField.parent.parent, undefined); assert.equal(field.boxedAt(0), oldMap); }); it("optional fields", () => { - const mutableMap = UnhydratedFlexTreeNode.getOrCreate( - context, - deepCopyMapTree(mapMapTree), - ); - const field = mutableMap.getBoxed(mapKey) as FlexTreeOptionalField; + const mutableMap = flexTreeFromCursor(context, cursorForMapTreeNode(mapMapTree)); + const field = mutableMap.getBoxed(mapKey) as UnhydratedOptionalField; const oldValue = field.boxedAt(0); const newValue = `new ${childValue}`; - field.editor.set({ ...mapChildMapTree, value: newValue }, false); + field.editor.set({ ...mapChildMapTree, value: newValue }); assert.equal(field.boxedAt(0)?.value, newValue); assert.notEqual(newValue, oldValue); - field.editor.set(undefined, false); + field.editor.set(undefined); assert.equal(field.boxedAt(0)?.value, undefined); }); describe("arrays", () => { it("insert and remove", () => { - const mutableFieldNode = UnhydratedFlexTreeNode.getOrCreate( + const mutableFieldNode = flexTreeFromCursor( context, - deepCopyMapTree(fieldNodeMapTree), + cursorForMapTreeNode(fieldNodeMapTree), ); const field = mutableFieldNode.getBoxed(EmptyKey); assert(field.is(FieldKinds.sequence)); - const values = () => Array.from(field.boxedIterator(), (n) => n.value); + const values = (): Value[] => Array.from(field.boxedIterator(), (n): Value => n.value); assert.deepEqual(values(), [childValue]); field.editor.insert(1, [ { ...mapChildMapTree, value: "c" }, @@ -259,10 +262,13 @@ describe("unhydratedFlexTree", () => { it("with a large sequence of new content", () => { // This exercises a special code path for inserting large arrays, since large arrays are treated differently to avoid overflow with `splice` + spread. - const mutableFieldNode = UnhydratedFlexTreeNode.getOrCreate(context, { - ...fieldNodeMapTree, - fields: new Map(), - }); + const mutableFieldNode = flexTreeFromCursor( + context, + cursorForMapTreeNode({ + ...fieldNodeMapTree, + fields: new Map(), + }), + ); const field = mutableFieldNode.getBoxed(EmptyKey); assert(field.is(FieldKinds.sequence)); const newContent: ExclusiveMapTree[] = []; diff --git a/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts b/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts index 2c1882bcf195..59aabcd2e9a3 100644 --- a/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts @@ -37,7 +37,6 @@ import { // eslint-disable-next-line import/no-internal-modules } from "../../simple-tree/schemaTypes.js"; import { - addDefaultsToMapTree, getPossibleTypes, mapTreeFromNodeData, type InsertableContent, @@ -46,9 +45,13 @@ import { import { brand } from "../../util/index.js"; import { MockNodeIdentifierManager, - type NodeIdentifierManager, + type FlexTreeHydratedContextMinimal, } from "../../feature-libraries/index.js"; import { validateUsageError } from "../utils.js"; +// eslint-disable-next-line import/no-internal-modules +import { UnhydratedFlexTreeNode } from "../../simple-tree/core/unhydratedFlexTree.js"; +// eslint-disable-next-line import/no-internal-modules +import { getUnhydratedContext } from "../../simple-tree/createContext.js"; describe("toMapTree", () => { let nodeKeyManager: MockNodeIdentifierManager; @@ -821,7 +824,7 @@ describe("toMapTree", () => { const tree = {}; - const actual = mapTreeFromNodeData(tree, schema, nodeKeyManager); + const actual = mapTreeFromNodeData(tree, schema); const expected: MapTree = { type: brand("test.object"), @@ -861,13 +864,38 @@ describe("toMapTree", () => { }); it("Populates a tree with defaults", () => { + const log: string[] = []; const defaultValue = 3; const constantProvider: ConstantFieldProvider = () => { - return defaultValue; + log.push("constant"); + return [ + new UnhydratedFlexTreeNode( + { + type: brand(numberSchema.identifier), + value: defaultValue, + }, + new Map(), + getUnhydratedContext(SchemaFactory.number), + ), + ]; }; - const contextualProvider: ContextualFieldProvider = (context: NodeIdentifierManager) => { - assert.equal(context, nodeKeyManager); - return defaultValue; + const contextualProvider: ContextualFieldProvider = ( + context: FlexTreeHydratedContextMinimal | "UseGlobalContext", + ) => { + log.push(typeof context === "string" ? context : `contextual`); + if (typeof context !== "string") { + assert.equal(context.nodeKeyManager, nodeKeyManager); + } + return [ + new UnhydratedFlexTreeNode( + { + type: brand(numberSchema.identifier), + value: defaultValue, + }, + new Map(), + getUnhydratedContext(SchemaFactory.number), + ), + ]; }; function createDefaultFieldProps(provider: FieldProvider): FieldProps { return { @@ -906,41 +934,65 @@ describe("toMapTree", () => { // Don't pass in a context let mapTree = mapTreeFromNodeData(nodeData, RootObject); - const getObject = () => mapTree.fields.get(brand("object"))?.[0]; - const getArray = () => mapTree.fields.get(brand("array"))?.[0].fields.get(EmptyKey); - const getMap = () => mapTree.fields.get(brand("map"))?.[0]; - const getConstantValue = (leafObject: MapTree | undefined) => - leafObject?.fields.get(brand("constantValue"))?.[0].value; - const getContextualValue = (leafObject: MapTree | undefined) => - leafObject?.fields.get(brand("contextualValue"))?.[0].value; + const getObject = () => mapTree.getBoxed("object").children[0]; + const getArray = () => mapTree.getBoxed("array").children[0].fields.get(EmptyKey); + const getMap = () => mapTree.getBoxed("map").children[0]; + const getConstantValue = (leafObject: UnhydratedFlexTreeNode | undefined) => + leafObject?.getBoxed("constantValue").children[0].value; + const getContextualValue = (leafObject: UnhydratedFlexTreeNode | undefined) => + leafObject?.getBoxed("contextualValue").children[0].value; // Assert that we've populated the constant defaults... assert.equal(getConstantValue(getObject()), defaultValue); - assert.equal(getConstantValue(getArray()?.[0]), defaultValue); - assert.equal(getConstantValue(getArray()?.[1]), defaultValue); - assert.equal(getConstantValue(getMap()?.fields.get(brand("a"))?.[0]), defaultValue); - assert.equal(getConstantValue(getMap()?.fields.get(brand("b"))?.[0]), defaultValue); + assert.equal(getConstantValue(getArray()?.children[0]), defaultValue); + assert.equal(getConstantValue(getArray()?.children[1]), defaultValue); + assert.equal( + getConstantValue(getMap()?.fields.get(brand("a"))?.children[0]), + defaultValue, + ); + assert.equal( + getConstantValue(getMap()?.fields.get(brand("b"))?.children[0]), + defaultValue, + ); // ...but not the contextual ones assert.equal(getContextualValue(getObject()), undefined); - assert.equal(getContextualValue(getArray()?.[0]), undefined); - assert.equal(getContextualValue(getArray()?.[1]), undefined); - assert.equal(getContextualValue(getMap()?.fields.get(brand("a"))?.[0]), undefined); - assert.equal(getContextualValue(getMap()?.fields.get(brand("b"))?.[0]), undefined); + assert.equal(getContextualValue(getArray()?.children[0]), undefined); + assert.equal(getContextualValue(getArray()?.children[1]), undefined); + assert.equal( + getContextualValue(getMap()?.fields.get(brand("a"))?.children[0]), + undefined, + ); + assert.equal( + getContextualValue(getMap()?.fields.get(brand("b"))?.children[0]), + undefined, + ); // This time, pass the context in - mapTree = mapTreeFromNodeData(nodeData, RootObject, nodeKeyManager); + mapTree = mapTreeFromNodeData(nodeData, RootObject); // Assert that all defaults are populated assert.equal(getConstantValue(getObject()), defaultValue); - assert.equal(getConstantValue(getArray()?.[0]), defaultValue); - assert.equal(getConstantValue(getArray()?.[1]), defaultValue); - assert.equal(getConstantValue(getMap()?.fields.get(brand("a"))?.[0]), defaultValue); - assert.equal(getConstantValue(getMap()?.fields.get(brand("b"))?.[0]), defaultValue); + assert.equal(getConstantValue(getArray()?.children[0]), defaultValue); + assert.equal(getConstantValue(getArray()?.children[1]), defaultValue); + assert.equal( + getConstantValue(getMap()?.fields.get(brand("a"))?.children[0]), + defaultValue, + ); + assert.equal( + getConstantValue(getMap()?.fields.get(brand("b"))?.children[0]), + defaultValue, + ); assert.equal(getContextualValue(getObject()), defaultValue); - assert.equal(getContextualValue(getArray()?.[0]), defaultValue); - assert.equal(getContextualValue(getArray()?.[1]), defaultValue); - assert.equal(getContextualValue(getMap()?.fields.get(brand("a"))?.[0]), defaultValue); - assert.equal(getContextualValue(getMap()?.fields.get(brand("b"))?.[0]), defaultValue); + assert.equal(getContextualValue(getArray()?.children[0]), defaultValue); + assert.equal(getContextualValue(getArray()?.children[1]), defaultValue); + assert.equal( + getContextualValue(getMap()?.fields.get(brand("a"))?.children[0]), + defaultValue, + ); + assert.equal( + getContextualValue(getMap()?.fields.get(brand("b"))?.children[0]), + defaultValue, + ); }); }); @@ -1338,11 +1390,19 @@ describe("toMapTree", () => { class Test extends f.object("test", { api: createFieldSchema(FieldKind.Required, [f.number], { key: "stored", - defaultProvider: getDefaultProvider(() => 5), + defaultProvider: getDefaultProvider(() => [ + new UnhydratedFlexTreeNode( + { + type: brand(numberSchema.identifier), + value: 5, + }, + new Map(), + getUnhydratedContext(SchemaFactory.number), + ), + ]), }), }) {} const m: ExclusiveMapTree = { type: brand(Test.identifier), fields: new Map() }; - addDefaultsToMapTree(m, Test, undefined); assert.deepEqual( m.fields, new Map([["stored", [{ type: f.number.identifier, fields: new Map(), value: 5 }]]]), diff --git a/packages/dds/tree/src/test/simple-tree/unhydratedNode.spec.ts b/packages/dds/tree/src/test/simple-tree/unhydratedNode.spec.ts index 672d6d2ae06d..ec9bb669337f 100644 --- a/packages/dds/tree/src/test/simple-tree/unhydratedNode.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/unhydratedNode.spec.ts @@ -27,6 +27,11 @@ import { TreeStatus } from "../../feature-libraries/index.js"; import { validateUsageError } from "../utils.js"; // eslint-disable-next-line import/no-internal-modules import { UnhydratedFlexTreeNode } from "../../simple-tree/core/unhydratedFlexTree.js"; +import { singleJsonCursor } from "../json/index.js"; +// eslint-disable-next-line import/no-internal-modules +import { flexTreeFromCursor } from "../../simple-tree/api/create.js"; +// eslint-disable-next-line import/no-internal-modules +import { getUnhydratedContext } from "../../simple-tree/createContext.js"; describe("Unhydrated nodes", () => { const schemaFactory = new SchemaFactory("undefined"); @@ -281,8 +286,13 @@ describe("Unhydrated nodes", () => { it("read constant defaulted properties", () => { const defaultValue = 3; - const constantProvider: ConstantFieldProvider = () => { - return defaultValue; + const constantProvider: ConstantFieldProvider = (): UnhydratedFlexTreeNode[] => { + return [ + flexTreeFromCursor( + getUnhydratedContext(SchemaFactory.number), + singleJsonCursor(defaultValue), + ), + ]; }; class HasDefault extends schemaFactory.object("DefaultingLeaf", { value: schemaFactory.optional( @@ -299,7 +309,12 @@ describe("Unhydrated nodes", () => { const defaultValue = 3; const contextualProvider: ContextualFieldProvider = (context: unknown) => { assert.notEqual(context, undefined); - return defaultValue; + return [ + flexTreeFromCursor( + getUnhydratedContext(SchemaFactory.number), + singleJsonCursor(defaultValue), + ), + ]; }; class HasDefault extends schemaFactory.object("DefaultingLeaf", { value: schemaFactory.optional( diff --git a/packages/dds/tree/src/test/utils.ts b/packages/dds/tree/src/test/utils.ts index cebdb6f83152..2b027335d155 100644 --- a/packages/dds/tree/src/test/utils.ts +++ b/packages/dds/tree/src/test/utils.ts @@ -1469,12 +1469,7 @@ export function fieldCursorFromInsertable< ? ImplicitFieldSchema : TSchema & ImplicitFieldSchema, data: InsertableField, - context?: NodeIdentifierManager | undefined, ): ITreeCursorSynchronous { - const mapTree = mapTreeFromNodeData( - data as InsertableField, - schema, - context, - ); + const mapTree = mapTreeFromNodeData(data as InsertableField, schema); return cursorForMapTreeField(mapTree === undefined ? [] : [mapTree]); } From 0e9c6d1fa5f930efc90dfa9f2ed1829e9b5239e3 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Wed, 28 May 2025 13:13:38 -0700 Subject: [PATCH 04/25] Fix test enumeration --- .../dds/tree/src/simple-tree/toMapTree.ts | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/packages/dds/tree/src/simple-tree/toMapTree.ts b/packages/dds/tree/src/simple-tree/toMapTree.ts index c2189f0e90d4..1225dea518ee 100644 --- a/packages/dds/tree/src/simple-tree/toMapTree.ts +++ b/packages/dds/tree/src/simple-tree/toMapTree.ts @@ -15,7 +15,7 @@ import { type ValueSchema, } from "../core/index.js"; import { FieldKinds, isTreeValue, valueSchemaAllows } from "../feature-libraries/index.js"; -import { brand, isReadonlyArray, hasSome, hasSingle } from "../util/index.js"; +import { brand, isReadonlyArray, hasSingle } from "../util/index.js"; import { nullSchema } from "./leafNodeSchema.js"; import { @@ -384,7 +384,7 @@ function objectToMapTree(data: FactoryContent, schema: TreeNodeSchema): FlexCont const context = getUnhydratedContext(schema).flexContext; for (const [key, fieldInfo] of schema.flexKeyMap) { - const value = checkFieldProperty(data, key); + const value = getFieldProperty(data, key); let children: UnhydratedFlexTreeNode[] | ContextualFieldProvider; if (value === undefined) { @@ -414,20 +414,27 @@ function objectToMapTree(data: FactoryContent, schema: TreeNodeSchema): FlexCont /** * Check {@link FactoryContentObject} for a property which could be store a field. + * + * If the property exists, return its value, otherwise returns undefined. * @remarks * The currently policy is to only consider own properties. * See {@link InsertableObjectFromSchemaRecord} for where this policy is documented in the public API. * - * Explicit undefined members are considered to exist, as long as they are own properties. + * Explicit undefined values are treated the same as missing properties to allow explicit use of undefined with defaulted identifiers. + * + * @privateRemarks + * If we ever want to have an optional field which defaults to something other than undefined, this will need changes. + * It would need to adjusting the handling of explicit undefined in contexts where undefined is allowed, and a default provider also exists. */ -function checkFieldProperty( +function getFieldProperty( data: FactoryContentObject, key: string | symbol, -): data is { - readonly [P in string]: InsertableContent | undefined; -} { +): InsertableContent | undefined { // This policy only allows own properties. - return Object.hasOwnProperty.call(data, key); + if (Object.hasOwnProperty.call(data, key)) { + return (data as Record)[key as string]; + } + return undefined; } function getType( @@ -442,10 +449,6 @@ function getType( )}.`, ); } - assert( - hasSome(possibleTypes), - 0x84e /* data is incompatible with all types allowed by the schema */, - ); if (!hasSingle(possibleTypes)) { throw new UsageError( `The provided data is compatible with more than one type allowed by the schema. @@ -584,11 +587,7 @@ function shallowCompatibilityTest( // If the schema has a required key which is not present in the input object, reject it. for (const [fieldKey, fieldSchema] of schema.fields) { if (fieldSchema.requiresValue) { - if (checkFieldProperty(data, fieldKey)) { - if (data[fieldKey] === undefined) { - return CompatibilityLevel.None; - } - } else { + if (getFieldProperty(data, fieldKey) === undefined) { return CompatibilityLevel.None; } } From 0d745768b700e330af48f046f219f681d0e56d9e Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Thu, 29 May 2025 17:01:44 -0700 Subject: [PATCH 05/25] Fix a bunch of tests --- packages/dds/tree/src/core/tree/mapTree.ts | 29 ++++++++--- .../dds/tree/src/feature-libraries/index.ts | 3 ++ .../src/feature-libraries/mapTreeCursor.ts | 27 +++++++++- .../dds/tree/src/simple-tree/api/create.ts | 6 ++- .../tree/src/simple-tree/api/treeNodeApi.ts | 49 +++++++++++++------ .../simple-tree/core/unhydratedFlexTree.ts | 36 +++++++++++--- .../src/simple-tree/prepareForInsertion.ts | 7 ++- .../core/unhydratedFlexTree.spec.ts | 21 ++++++-- .../src/test/simple-tree/toMapTree.spec.ts | 47 +++++++++--------- .../test/simple-tree/unhydratedNode.spec.ts | 20 ++++++-- packages/dds/tree/src/test/utils.ts | 33 +++++++++---- 11 files changed, 203 insertions(+), 75 deletions(-) diff --git a/packages/dds/tree/src/core/tree/mapTree.ts b/packages/dds/tree/src/core/tree/mapTree.ts index 9e49f1795f6f..4b200a988120 100644 --- a/packages/dds/tree/src/core/tree/mapTree.ts +++ b/packages/dds/tree/src/core/tree/mapTree.ts @@ -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"; @@ -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 { + 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()); newField.push(childClone); nexts.push([childClone.fields, child.fields]); } @@ -55,8 +73,5 @@ export function deepCopyMapTree(mapTree: MapTree): ExclusiveMapTree { } } - return { - ...mapTree, - fields: rootFields, - }; + return create(mapTree, rootFields); } diff --git a/packages/dds/tree/src/feature-libraries/index.ts b/packages/dds/tree/src/feature-libraries/index.ts index 436a0174dfb7..93807d9c19f8 100644 --- a/packages/dds/tree/src/feature-libraries/index.ts +++ b/packages/dds/tree/src/feature-libraries/index.ts @@ -15,6 +15,9 @@ export { cursorForMapTreeNode, mapTreeFromCursor, mapTreeFieldFromCursor, + type MinimalMapTreeNodeView, + mapTreeFieldsWithField, + mapTreeWithField, } from "./mapTreeCursor.js"; export { buildForest } from "./object-forest/index.js"; export { diff --git a/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts b/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts index 4e17a08b0f46..8179f5ef533d 100644 --- a/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts +++ b/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts @@ -17,6 +17,7 @@ import { detachedFieldAsKey, mapCursorField, rootField, + rootFieldKey, } from "../core/index.js"; import { @@ -29,6 +30,9 @@ import { import type { requireAssignableTo } from "../util/index.js"; export interface MapTreeNodeViewGeneric extends NodeData { + /** + * The non-empty fields on this node. + */ readonly fields: Pick< ReadonlyMap>, typeof Symbol.iterator | "get" | "size" | "keys" @@ -40,6 +44,9 @@ export type MapTreeFieldViewGeneric = Pick< typeof Symbol.iterator | "length" >; +/** + * Like {@link MapTree} but with the minimal properties needed for reading. + */ export interface MinimalMapTreeNodeView extends MapTreeNodeViewGeneric {} @@ -58,6 +65,24 @@ export function cursorForMapTreeNode>( return stackTreeNodeCursor(adapterTyped, root); } +export function mapTreeWithField( + children: ExclusiveMapTree[], + key = rootFieldKey, + type = aboveRootPlaceholder, +): MapTree { + return { + type, + fields: mapTreeFieldsWithField(children, key), + }; +} + +export function mapTreeFieldsWithField( + children: T, + key: FieldKey, +): Map { + return new Map(children.length === 0 ? [] : [[key, children]]); +} + /** * Returns an {@link ITreeCursorSynchronous} in fields mode for a MapTree field. */ @@ -69,7 +94,7 @@ export function cursorForMapTreeField>( const adapterTyped = adapter as unknown as CursorAdapter; const dummyRoot: MapTreeNodeViewGeneric = { type: aboveRootPlaceholder, - fields: new Map([[key, root]]), + fields: mapTreeFieldsWithField(root, key), }; return stackTreeFieldCursor(adapterTyped, dummyRoot as T, detachedField); } diff --git a/packages/dds/tree/src/simple-tree/api/create.ts b/packages/dds/tree/src/simple-tree/api/create.ts index 98f4b9b75589..2955a1d7e8be 100644 --- a/packages/dds/tree/src/simple-tree/api/create.ts +++ b/packages/dds/tree/src/simple-tree/api/create.ts @@ -100,5 +100,9 @@ export function flexTreeFromCursor( ), ]), ); - return new UnhydratedFlexTreeNode({ type: cursor.type }, fields, context); + return new UnhydratedFlexTreeNode( + { type: cursor.type, value: cursor.value }, + fields, + context, + ); } diff --git a/packages/dds/tree/src/simple-tree/api/treeNodeApi.ts b/packages/dds/tree/src/simple-tree/api/treeNodeApi.ts index 13052aee225d..3b222ba606ed 100644 --- a/packages/dds/tree/src/simple-tree/api/treeNodeApi.ts +++ b/packages/dds/tree/src/simple-tree/api/treeNodeApi.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { assert, oob, fail } from "@fluidframework/core-utils/internal"; +import { assert, oob, fail, unreachableCase } from "@fluidframework/core-utils/internal"; import { EmptyKey, rootFieldKey } from "../../core/index.js"; import { type TreeStatus, isTreeValue, FieldKinds } from "../../feature-libraries/index.js"; @@ -39,6 +39,7 @@ import { } from "../core/index.js"; import type { TreeChangeEvents } from "./treeChangeEvents.js"; import { isObjectNodeSchema } from "../node-kinds/index.js"; +import { getTreeNodeForField } from "../getTreeNodeForField.js"; /** * Provides various functions for analyzing {@link TreeNode}s. @@ -302,23 +303,39 @@ export function getIdentifierFromNode( return undefined; case 1: { const key = identifierFieldKeys[0] ?? oob(); - const identifier = flexNode.tryGetField(key)?.boxedAt(0); - assert( - identifier?.context.isHydrated() === true, - 0xa27 /* Expected hydrated identifier */, - ); - const identifierValue = identifier.value as string; + const identifierField = flexNode.tryGetField(key); + assert(identifierField !== undefined, "missing identifier field"); + const identifierValue = getTreeNodeForField(identifierField); + assert(typeof identifierValue === "string", "identifier not a string"); - if (compression === "preferCompressed") { - const localNodeKey = - identifier.context.nodeKeyManager.tryLocalizeNodeIdentifier(identifierValue); - return localNodeKey !== undefined ? extractFromOpaque(localNodeKey) : identifierValue; - } else if (compression === "compressed") { - const localNodeKey = - identifier.context.nodeKeyManager.tryLocalizeNodeIdentifier(identifierValue); - return localNodeKey !== undefined ? extractFromOpaque(localNodeKey) : undefined; + const context = flexNode.context; + switch (compression) { + case "preferCompressed": { + if (context.isHydrated()) { + const localNodeKey = + context.nodeKeyManager.tryLocalizeNodeIdentifier(identifierValue); + return localNodeKey !== undefined + ? extractFromOpaque(localNodeKey) + : identifierValue; + } else { + return identifierValue; + } + } + case "compressed": { + if (context.isHydrated()) { + const localNodeKey = + context.nodeKeyManager.tryLocalizeNodeIdentifier(identifierValue); + return localNodeKey !== undefined ? extractFromOpaque(localNodeKey) : undefined; + } else { + return undefined; + } + } + case "uncompressed": { + return identifierValue; + } + default: + unreachableCase(compression); } - return identifierValue; } default: throw new UsageError( diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index 66d1c4be69fa..f3b59043782b 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -22,6 +22,7 @@ import { type TreeNodeSchemaIdentifier, type TreeNodeStoredSchema, type TreeStoredSchema, + type TreeValue, type Value, } from "../../core/index.js"; import { @@ -108,14 +109,32 @@ export class UnhydratedFlexTreeNode */ public constructor( public readonly data: NodeData, - public readonly fields: Map, + private readonly fieldsAll: Map, public readonly simpleContext: Context, ) { - for (const [_key, field] of this.fields) { + for (const [_key, field] of this.fieldsAll) { field.parent = this; } } + /** + * The non-empty fields on this node. + */ + public get fields(): ReadonlyMap { + // TODO: doing this copy on every access is inefficient. SOme caching+invalidation or removal of the need for a separate map would be better. + const entries = [...this.fieldsAll.entries()].filter(([, field]) => field.length > 0); + return new Map(entries); + } + + /** + * Gets all fields, without filtering out empty ones. + * @remarks + * This avoids forcing the evaluating of pending defaults in the fields, and also saves a copy on access. + */ + public get allFieldsLazy(): ReadonlyMap { + return this.fieldsAll; + } + public get type(): TreeNodeSchemaIdentifier { return this.data.type; } @@ -125,7 +144,7 @@ export class UnhydratedFlexTreeNode } private getOrCreateField(key: FieldKey): UnhydratedFlexTreeField { - return getOrCreate(this.fields, key, () => { + return getOrCreate(this.fieldsAll, key, () => { const stored = this.storedSchema.getFieldSchema(key).kind; const field = createField(this.context, stored, key, []); field.parent = this; @@ -146,7 +165,7 @@ export class UnhydratedFlexTreeNode if (parent !== undefined) { assert(index !== undefined, 0xa08 /* Expected index */); if (this.location !== unparentedLocation) { - throw new UsageError("A node may not be inserted if it's already in a tree"); + throw new UsageError("A node may not be in more than one place in the tree"); } let unhydratedNode: UnhydratedFlexTreeNode | undefined = parent.parent; while (unhydratedNode !== undefined) { @@ -393,8 +412,13 @@ export class UnhydratedFlexTreeField } /** Unboxes leaf nodes to their values */ - protected unboxed(index: number): UnhydratedFlexTreeNode { - return this.children[index] ?? oob(); + protected unboxed(index: number): TreeValue | UnhydratedFlexTreeNode { + const child = this.children[index] ?? oob(); + const value = child.value; + if (value !== undefined) { + return value; + } + return child; } } diff --git a/packages/dds/tree/src/simple-tree/prepareForInsertion.ts b/packages/dds/tree/src/simple-tree/prepareForInsertion.ts index ec22a7155d23..ece373af8f72 100644 --- a/packages/dds/tree/src/simple-tree/prepareForInsertion.ts +++ b/packages/dds/tree/src/simple-tree/prepareForInsertion.ts @@ -131,11 +131,14 @@ function validateAndPrepare( mapTrees: readonly UnhydratedFlexTreeNode[], ): void { if (hydratedData !== undefined) { + // Prepare content before validating side this populated defaults using the provided context rather than the global context. + // This ensures that when validation requests identifiers (or any other contextual defaults), + // they were already creating used the more specific context we have access to from `hydratedData`. + prepareContentForHydration(mapTrees, hydratedData.checkout.forest, hydratedData); if (schemaAndPolicy.policy.validateSchema === true) { const maybeError = isFieldInSchema(mapTrees, fieldSchema, schemaAndPolicy); inSchemaOrThrow(maybeError); } - prepareContentForHydration(mapTrees, hydratedData.checkout.forest, hydratedData); } } @@ -233,7 +236,7 @@ function walkMapTree( } } - for (const [key, field] of node.fields) { + for (const [key, field] of node.allFieldsLazy) { field.fillPendingDefaults(context); for (const [i, child] of field.children.entries()) { nexts.push([ diff --git a/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts b/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts index 0ba7ad4da8e5..c96ffc1f16b5 100644 --- a/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts @@ -23,6 +23,7 @@ import { SchemaFactory, stringSchema } from "../../../simple-tree/index.js"; import { getUnhydratedContext } from "../../../simple-tree/createContext.js"; // eslint-disable-next-line import/no-internal-modules import { flexTreeFromCursor } from "../../../simple-tree/api/create.js"; +import { expectEqualCursors } from "../../utils.js"; describe("unhydratedFlexTree", () => { // #region The schema used in this test suite @@ -135,10 +136,22 @@ describe("unhydratedFlexTree", () => { it("can get the children of object nodes", () => { assert.equal(object.getBoxed("map").key, "map"); - assert.equal(object.tryGetField(objectMapKey)?.boxedAt(0), map); - assert.equal(object.tryGetField(objectFieldNodeKey)?.boxedAt(0), arrayNode); - assert.equal(object.getBoxed(objectMapKey).boxedAt(0), map); - assert.equal(object.getBoxed(objectFieldNodeKey).boxedAt(0), arrayNode); + expectEqualCursors( + object.tryGetField(objectMapKey)?.boxedAt(0)?.borrowCursor(), + map.borrowCursor(), + ); + expectEqualCursors( + object.tryGetField(objectFieldNodeKey)?.boxedAt(0)?.borrowCursor(), + arrayNode.borrowCursor(), + ); + expectEqualCursors( + object.getBoxed(objectMapKey).boxedAt(0)?.borrowCursor(), + map.borrowCursor(), + ); + expectEqualCursors( + object.getBoxed(objectFieldNodeKey).boxedAt(0)?.borrowCursor(), + arrayNode.borrowCursor(), + ); assert.equal(object.tryGetField(brand("unknown key")), undefined); assert.equal(object.getBoxed("unknown key").length, 0); assert.equal([...object.boxedIterator()].length, 2); diff --git a/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts b/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts index 59aabcd2e9a3..83ef4bcd840e 100644 --- a/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts @@ -11,6 +11,7 @@ import { } from "@fluidframework/test-runtime-utils/internal"; import { + deepCopyMapTree, EmptyKey, type ExclusiveMapTree, type FieldKey, @@ -54,11 +55,6 @@ import { UnhydratedFlexTreeNode } from "../../simple-tree/core/unhydratedFlexTre import { getUnhydratedContext } from "../../simple-tree/createContext.js"; describe("toMapTree", () => { - let nodeKeyManager: MockNodeIdentifierManager; - beforeEach(() => { - nodeKeyManager = new MockNodeIdentifierManager(); - }); - it("string", () => { const schemaFactory = new SchemaFactory("test"); const tree = "Hello world"; @@ -71,7 +67,7 @@ describe("toMapTree", () => { fields: new Map(), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("null", () => { @@ -86,7 +82,7 @@ describe("toMapTree", () => { fields: new Map(), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("handle", () => { @@ -103,7 +99,7 @@ describe("toMapTree", () => { fields: new Map(), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("recursive", () => { @@ -153,7 +149,7 @@ describe("toMapTree", () => { ]), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Fails when referenced schema has not yet been instantiated", () => { @@ -204,7 +200,7 @@ describe("toMapTree", () => { fields: new Map(), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Simple array", () => { @@ -245,7 +241,7 @@ describe("toMapTree", () => { ]), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Complex array", () => { @@ -311,7 +307,7 @@ describe("toMapTree", () => { ]), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Recursive array", () => { @@ -409,7 +405,7 @@ describe("toMapTree", () => { fields: new Map(), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Simple map", () => { @@ -449,7 +445,7 @@ describe("toMapTree", () => { ]), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Complex Map", () => { @@ -529,7 +525,7 @@ describe("toMapTree", () => { ]), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Undefined map entries are omitted", () => { @@ -559,7 +555,7 @@ describe("toMapTree", () => { ]), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Throws on schema-incompatible entries", () => { @@ -613,7 +609,7 @@ describe("toMapTree", () => { fields: new Map(), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Simple object", () => { @@ -656,7 +652,7 @@ describe("toMapTree", () => { ]), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Complex object", () => { @@ -735,7 +731,7 @@ describe("toMapTree", () => { ]), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Undefined properties are omitted", () => { @@ -764,7 +760,7 @@ describe("toMapTree", () => { ]), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Object with stored field keys specified", () => { @@ -813,10 +809,11 @@ describe("toMapTree", () => { ]), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Populates identifier field with the default identifier provider", () => { + const nodeKeyManager = new MockNodeIdentifierManager(); const schemaFactory = new SchemaFactory("test"); const schema = schemaFactory.object("object", { a: schemaFactory.identifier, @@ -842,7 +839,7 @@ describe("toMapTree", () => { ]), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Populates optional field with the default optional provider.", () => { @@ -864,6 +861,8 @@ describe("toMapTree", () => { }); it("Populates a tree with defaults", () => { + const nodeKeyManager = new MockNodeIdentifierManager(); + const log: string[] = []; const defaultValue = 3; const constantProvider: ConstantFieldProvider = () => { @@ -1174,7 +1173,7 @@ describe("toMapTree", () => { ]), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("ambiguous unions", () => { @@ -1294,7 +1293,7 @@ describe("toMapTree", () => { ]), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Array containing `undefined` (throws if fallback type when not allowed by the schema)", () => { diff --git a/packages/dds/tree/src/test/simple-tree/unhydratedNode.spec.ts b/packages/dds/tree/src/test/simple-tree/unhydratedNode.spec.ts index ec9bb669337f..61b25ea11d62 100644 --- a/packages/dds/tree/src/test/simple-tree/unhydratedNode.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/unhydratedNode.spec.ts @@ -284,6 +284,15 @@ describe("Unhydrated nodes", () => { assertArray(); }); + it("flexTreeFromCursor", () => { + const tree = flexTreeFromCursor( + getUnhydratedContext(SchemaFactory.number), + singleJsonCursor(1), + ); + + assert.equal(tree.value, 1); + }); + it("read constant defaulted properties", () => { const defaultValue = 3; const constantProvider: ConstantFieldProvider = (): UnhydratedFlexTreeNode[] => { @@ -304,11 +313,10 @@ describe("Unhydrated nodes", () => { assert.equal(defaultingLeaf.value, defaultValue); }); - // TODO: Fail instead of returning undefined, as is the case for identifiers. it("read undefined for contextual defaulted properties", () => { const defaultValue = 3; const contextualProvider: ContextualFieldProvider = (context: unknown) => { - assert.notEqual(context, undefined); + assert.equal(context, "UseGlobalContext"); return [ flexTreeFromCursor( getUnhydratedContext(SchemaFactory.number), @@ -323,7 +331,7 @@ describe("Unhydrated nodes", () => { ), }) {} const defaultingLeaf = new HasDefault({ value: undefined }); - assert.equal(defaultingLeaf.value, undefined); + assert.equal(defaultingLeaf.value, defaultValue); }); it("read manually provided identifiers", () => { @@ -353,7 +361,11 @@ describe("Unhydrated nodes", () => { const id = "my identifier"; const object = new TestObjectWithId({ id, autoId: undefined }); - assert.deepEqual(Object.entries(object), [["id", id]]); + assert.deepEqual(Object.entries(object), [ + ["id", id], + ["autoId", object.autoId], + ]); + assert(isStableId(object.autoId)); }); it("cannot be used twice in the same tree", () => { diff --git a/packages/dds/tree/src/test/utils.ts b/packages/dds/tree/src/test/utils.ts index 2b027335d155..5711594c350b 100644 --- a/packages/dds/tree/src/test/utils.ts +++ b/packages/dds/tree/src/test/utils.ts @@ -94,8 +94,6 @@ import { type NormalizedFieldUpPath, type ExclusiveMapTree, type MapTree, - aboveRootPlaceholder, - rootFieldKey, SchemaVersion, } from "../core/index.js"; import { typeboxValidator } from "../external-utilities/index.js"; @@ -115,6 +113,10 @@ import { cursorForJsonableTreeField, chunkFieldSingle, makeSchemaCodec, + mapTreeWithField, + type MinimalMapTreeNodeView, + jsonableTreeFromCursor, + cursorForMapTreeNode, } from "../feature-libraries/index.js"; import { type CheckoutEvents, @@ -821,10 +823,7 @@ function createCheckoutWithContent( }, ): { checkout: TreeCheckout; logger: IMockLoggerExt } { const fieldCursor = normalizeNewFieldContent(content.initialTree); - const roots: MapTree = { - type: aboveRootPlaceholder, - fields: new Map([[rootFieldKey, mapTreeFieldFromCursor(fieldCursor)]]), - }; + const roots: MapTree = mapTreeWithField(mapTreeFieldFromCursor(fieldCursor)); const schema = new TreeStoredSchemaRepository(content.schema); const forest = buildTestForest({ additionalAsserts: args?.additionalAsserts ?? true, @@ -892,10 +891,7 @@ export function buildTestForest(options: { export function forestWithContent(content: TreeStoredContent): IEditableForest { const fieldCursor = normalizeNewFieldContent(content.initialTree); - const roots: MapTree = { - type: aboveRootPlaceholder, - fields: new Map([[rootFieldKey, mapTreeFieldFromCursor(fieldCursor)]]), - }; + const roots: MapTree = mapTreeWithField(mapTreeFieldFromCursor(fieldCursor)); const forest = buildTestForest({ additionalAsserts: true, schema: new TreeStoredSchemaRepository(content.schema), @@ -963,6 +959,23 @@ export function expectJsonTree( } } +export function expectEqualMapTreeViews( + actual: MinimalMapTreeNodeView, + expected: MinimalMapTreeNodeView, +): void { + expectEqualCursors(cursorForMapTreeNode(actual), cursorForMapTreeNode(expected)); +} + +export function expectEqualCursors( + actual: ITreeCursorSynchronous | undefined, + expected: ITreeCursorSynchronous, +): void { + assert(actual !== undefined); + const actualJson = jsonableTreeFromCursor(actual); + const expectedJson = jsonableTreeFromCursor(expected); + assert.deepEqual(actualJson, expectedJson); +} + export function expectNoRemovedRoots(tree: ITreeCheckout): void { assert(tree.getRemovedRoots().length === 0); } From e812c58e68c4bfacc801cdb7216f6264701149f2 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Fri, 30 May 2025 10:32:18 -0700 Subject: [PATCH 06/25] Fix more tests --- .../src/simple-tree/prepareForInsertion.ts | 4 +- .../src/test/simple-tree/api/create.spec.ts | 13 +- .../src/test/simple-tree/toMapTree.spec.ts | 247 +++++++----------- 3 files changed, 103 insertions(+), 161 deletions(-) diff --git a/packages/dds/tree/src/simple-tree/prepareForInsertion.ts b/packages/dds/tree/src/simple-tree/prepareForInsertion.ts index ece373af8f72..af1db4c16888 100644 --- a/packages/dds/tree/src/simple-tree/prepareForInsertion.ts +++ b/packages/dds/tree/src/simple-tree/prepareForInsertion.ts @@ -180,11 +180,13 @@ const placeholderKey: DetachedField & FieldKey = brand("placeholder" as const); * @remarks If the content tree contains any proxies, this function must be called just prior to inserting the content into the tree. * Specifically, no other content may be inserted into the tree between the invocation of this function and the insertion of `content`. * The insertion of `content` must occur or else this function will cause memory leaks. + * + * Exported fot testing purposes: otherwise should not be used outside this module. * @param content - the content subsequence to be inserted, of which might deeply contain {@link TreeNode}s which need to be hydrated. * @param forest - the forest the content is being inserted into. * See {@link extractFactoryContent} for more details. */ -function prepareContentForHydration( +export function prepareContentForHydration( content: readonly UnhydratedFlexTreeNode[], forest: IForestSubscription, context: FlexTreeHydratedContextMinimal, diff --git a/packages/dds/tree/src/test/simple-tree/api/create.spec.ts b/packages/dds/tree/src/test/simple-tree/api/create.spec.ts index a93d91266a5d..81c77de90e51 100644 --- a/packages/dds/tree/src/test/simple-tree/api/create.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/api/create.spec.ts @@ -13,6 +13,7 @@ import { import { SchemaFactory } from "../../../simple-tree/index.js"; import { validateUsageError } from "../../utils.js"; import { singleJsonCursor } from "../../json/index.js"; +import { validateAssertionError } from "@fluidframework/test-runtime-utils/internal"; describe("simple-tree create", () => { describe("createFromCursor", () => { @@ -21,10 +22,20 @@ describe("simple-tree create", () => { createFromCursor(SchemaFactory.string, cursor); }); - it("Failure", () => { + it("Failure: unknown schema", () => { const cursor = singleJsonCursor("Hello world"); assert.throws( () => createFromCursor(SchemaFactory.number, cursor), + (e: Error) => validateAssertionError(e, /missing schema/), + ); + }); + + it("Failure: out of schema", () => { + const factory = new SchemaFactory("test"); + class Obj extends factory.object("Obj", { x: SchemaFactory.string }) {} + const cursor = singleJsonCursor("Hello world"); + assert.throws( + () => createFromCursor(Obj, cursor), validateUsageError(/does not conform to schema/), ); }); diff --git a/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts b/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts index 83ef4bcd840e..153a8b8115f3 100644 --- a/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts @@ -10,15 +10,10 @@ import { validateAssertionError, } from "@fluidframework/test-runtime-utils/internal"; -import { - deepCopyMapTree, - EmptyKey, - type ExclusiveMapTree, - type FieldKey, - type MapTree, -} from "../../core/index.js"; +import { deepCopyMapTree, EmptyKey, type FieldKey, type MapTree } from "../../core/index.js"; import { booleanSchema, + getTreeNodeForField, handleSchema, nullSchema, numberSchema, @@ -28,10 +23,6 @@ import { type ValidateRecursiveSchema, } from "../../simple-tree/index.js"; import { - type ContextualFieldProvider, - type ConstantFieldProvider, - type FieldProvider, - type FieldProps, createFieldSchema, FieldKind, getDefaultProvider, @@ -53,6 +44,11 @@ import { validateUsageError } from "../utils.js"; import { UnhydratedFlexTreeNode } from "../../simple-tree/core/unhydratedFlexTree.js"; // eslint-disable-next-line import/no-internal-modules import { getUnhydratedContext } from "../../simple-tree/createContext.js"; +// eslint-disable-next-line import/no-internal-modules +import { getKernel } from "../../simple-tree/core/index.js"; +import { hydrate } from "./utils.js"; +// eslint-disable-next-line import/no-internal-modules +import { prepareContentForHydration } from "../../simple-tree/prepareForInsertion.js"; describe("toMapTree", () => { it("string", () => { @@ -362,7 +358,7 @@ describe("toMapTree", () => { ]), }; - assert.deepEqual(actual, expected); + assert.deepEqual(deepCopyMapTree(actual), expected); }); it("Throws on `undefined` entries when null is not allowed", () => { @@ -822,6 +818,15 @@ describe("toMapTree", () => { const tree = {}; const actual = mapTreeFromNodeData(tree, schema); + const dummy = hydrate(schema, {}); + const dummyContext = getKernel(dummy).context.flexContext; + assert(dummyContext.isHydrated()); + // Do the default allocation using this context + const context: FlexTreeHydratedContextMinimal = { + checkout: dummyContext.checkout, + nodeKeyManager, + }; + prepareContentForHydration([actual], context.checkout.forest, context); const expected: MapTree = { type: brand("test.object"), @@ -857,141 +862,7 @@ describe("toMapTree", () => { fields: new Map(), }; - assert.deepEqual(actual, expected); - }); - - it("Populates a tree with defaults", () => { - const nodeKeyManager = new MockNodeIdentifierManager(); - - const log: string[] = []; - const defaultValue = 3; - const constantProvider: ConstantFieldProvider = () => { - log.push("constant"); - return [ - new UnhydratedFlexTreeNode( - { - type: brand(numberSchema.identifier), - value: defaultValue, - }, - new Map(), - getUnhydratedContext(SchemaFactory.number), - ), - ]; - }; - const contextualProvider: ContextualFieldProvider = ( - context: FlexTreeHydratedContextMinimal | "UseGlobalContext", - ) => { - log.push(typeof context === "string" ? context : `contextual`); - if (typeof context !== "string") { - assert.equal(context.nodeKeyManager, nodeKeyManager); - } - return [ - new UnhydratedFlexTreeNode( - { - type: brand(numberSchema.identifier), - value: defaultValue, - }, - new Map(), - getUnhydratedContext(SchemaFactory.number), - ), - ]; - }; - function createDefaultFieldProps(provider: FieldProvider): FieldProps { - return { - // By design, the public `DefaultProvider` type cannot be casted to, so we must disable type checking with `any`. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - defaultProvider: provider as any, - }; - } - - const schemaFactory = new SchemaFactory("test"); - class LeafObject extends schemaFactory.object("Leaf", { - constantValue: schemaFactory.optional( - schemaFactory.number, - createDefaultFieldProps(constantProvider), - ), - contextualValue: schemaFactory.optional( - schemaFactory.number, - createDefaultFieldProps(contextualProvider), - ), - }) {} - class RootObject extends schemaFactory.object("Root", { - object: schemaFactory.required(LeafObject), - array: schemaFactory.array(LeafObject), - map: schemaFactory.map(LeafObject), - }) {} - - const nodeData = { - object: {}, - array: [{}, {}], - map: new Map([ - ["a", {}], - ["b", {}], - ]), - }; - - // Don't pass in a context - let mapTree = mapTreeFromNodeData(nodeData, RootObject); - - const getObject = () => mapTree.getBoxed("object").children[0]; - const getArray = () => mapTree.getBoxed("array").children[0].fields.get(EmptyKey); - const getMap = () => mapTree.getBoxed("map").children[0]; - const getConstantValue = (leafObject: UnhydratedFlexTreeNode | undefined) => - leafObject?.getBoxed("constantValue").children[0].value; - const getContextualValue = (leafObject: UnhydratedFlexTreeNode | undefined) => - leafObject?.getBoxed("contextualValue").children[0].value; - - // Assert that we've populated the constant defaults... - assert.equal(getConstantValue(getObject()), defaultValue); - assert.equal(getConstantValue(getArray()?.children[0]), defaultValue); - assert.equal(getConstantValue(getArray()?.children[1]), defaultValue); - assert.equal( - getConstantValue(getMap()?.fields.get(brand("a"))?.children[0]), - defaultValue, - ); - assert.equal( - getConstantValue(getMap()?.fields.get(brand("b"))?.children[0]), - defaultValue, - ); - // ...but not the contextual ones - assert.equal(getContextualValue(getObject()), undefined); - assert.equal(getContextualValue(getArray()?.children[0]), undefined); - assert.equal(getContextualValue(getArray()?.children[1]), undefined); - assert.equal( - getContextualValue(getMap()?.fields.get(brand("a"))?.children[0]), - undefined, - ); - assert.equal( - getContextualValue(getMap()?.fields.get(brand("b"))?.children[0]), - undefined, - ); - - // This time, pass the context in - mapTree = mapTreeFromNodeData(nodeData, RootObject); - - // Assert that all defaults are populated - assert.equal(getConstantValue(getObject()), defaultValue); - assert.equal(getConstantValue(getArray()?.children[0]), defaultValue); - assert.equal(getConstantValue(getArray()?.children[1]), defaultValue); - assert.equal( - getConstantValue(getMap()?.fields.get(brand("a"))?.children[0]), - defaultValue, - ); - assert.equal( - getConstantValue(getMap()?.fields.get(brand("b"))?.children[0]), - defaultValue, - ); - assert.equal(getContextualValue(getObject()), defaultValue); - assert.equal(getContextualValue(getArray()?.children[0]), defaultValue); - assert.equal(getContextualValue(getArray()?.children[1]), defaultValue); - assert.equal( - getContextualValue(getMap()?.fields.get(brand("a"))?.children[0]), - defaultValue, - ); - assert.equal( - getContextualValue(getMap()?.fields.get(brand("b"))?.children[0]), - defaultValue, - ); + assert.deepEqual(deepCopyMapTree(actual), expected); }); }); @@ -1382,30 +1253,88 @@ describe("toMapTree", () => { }); }); - describe("addDefaultsToMapTree", () => { - it("custom stored key", () => { - const f = new SchemaFactory("test"); + describe("defaults", () => { + const f = new SchemaFactory("test"); + it("ConstantFieldProvider", () => { class Test extends f.object("test", { - api: createFieldSchema(FieldKind.Required, [f.number], { + api: createFieldSchema(FieldKind.Required, [f.string], { key: "stored", defaultProvider: getDefaultProvider(() => [ new UnhydratedFlexTreeNode( { - type: brand(numberSchema.identifier), - value: 5, + type: brand(stringSchema.identifier), + value: "x", }, new Map(), - getUnhydratedContext(SchemaFactory.number), + getUnhydratedContext(SchemaFactory.string), ), ]), }), }) {} - const m: ExclusiveMapTree = { type: brand(Test.identifier), fields: new Map() }; - assert.deepEqual( - m.fields, - new Map([["stored", [{ type: f.number.identifier, fields: new Map(), value: 5 }]]]), - ); + + const node = mapTreeFromNodeData({}, Test); + const field = node.getBoxed("stored"); + assert(!field.pendingDefault); + const read = getTreeNodeForField(field); + assert.equal(read, "x"); + }); + + describe("ContextualFieldProvider", () => { + class Test extends f.object("test", { + api: createFieldSchema(FieldKind.Required, [f.string], { + key: "stored", + defaultProvider: getDefaultProvider((context) => [ + new UnhydratedFlexTreeNode( + { + type: brand(stringSchema.identifier), + value: context === "UseGlobalContext" ? "global" : "contextual", + }, + new Map(), + getUnhydratedContext(SchemaFactory.string), + ), + ]), + }), + }) {} + + it("Implicit read with global context", () => { + const node = mapTreeFromNodeData({}, Test); + const field = node.getBoxed("stored"); + assert(field.pendingDefault); + const read = getTreeNodeForField(field); + assert(!field.pendingDefault); + assert.equal(read, "global"); + }); + + it("Explicit populate with valid context", () => { + const node = mapTreeFromNodeData({}, Test); + const field = node.getBoxed("stored"); + assert(field.pendingDefault); + const dummy = hydrate(Test, new Test({ api: "dummy" })); + const context = getKernel(dummy).context.flexContext; + assert(context.isHydrated()); + field.fillPendingDefaults(context); + const read = getTreeNodeForField(field); + assert(!field.pendingDefault); + assert.equal(read, "contextual"); + }); + + // Uses a context which does not know about the schema being used. + // This helps ensure that creation of invalid defaults won't assert (a usage error would be fine). + // This test does not run the schema validation, which happens after defaults are populated, so it simply must either usage error or complete. + it("Explicit populate with invalid context", () => { + const node = mapTreeFromNodeData({}, Test); + const field = node.getBoxed("stored"); + assert(field.pendingDefault); + class Test2 extends f.object("test2", {}) {} + const dummy = hydrate(Test2, new Test2({})); + const context = getKernel(dummy).context.flexContext; + assert(context.isHydrated()); + field.fillPendingDefaults(context); + const read = getTreeNodeForField(field); + assert(!field.pendingDefault); + assert.equal(read, "contextual"); + }); }); }); }); From 1c6350cf983b2a1ae1cae8b9e2a4ae49d41d4f96 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Fri, 30 May 2025 11:23:12 -0700 Subject: [PATCH 07/25] Fix more tests, mostly verbose tree validation --- .../dds/tree/src/simple-tree/api/create.ts | 5 +- .../tree/src/simple-tree/api/customTree.ts | 7 +++ .../tree/src/simple-tree/api/verboseTree.ts | 27 +++++----- .../simple-tree/api/integrationTests.spec.ts | 2 +- .../test/simple-tree/api/treeNodeApi.spec.ts | 53 +++++++++++++++++-- 5 files changed, 75 insertions(+), 19 deletions(-) diff --git a/packages/dds/tree/src/simple-tree/api/create.ts b/packages/dds/tree/src/simple-tree/api/create.ts index 2955a1d7e8be..bf1ac50f4cf8 100644 --- a/packages/dds/tree/src/simple-tree/api/create.ts +++ b/packages/dds/tree/src/simple-tree/api/create.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { assert, fail } from "@fluidframework/core-utils/internal"; +import { assert } from "@fluidframework/core-utils/internal"; import { CursorLocationType, @@ -31,6 +31,7 @@ import type { SimpleNodeSchema, SimpleNodeSchemaBase } from "../simpleSchema.js" // eslint-disable-next-line import/no-internal-modules import { createField } from "../core/unhydratedFlexTree.js"; import { getStoredSchema } from "../toStoredSchema.js"; +import { unknownTypeError } from "./customTree.js"; /** * Creates an unhydrated simple-tree field from a cursor in nodes mode. @@ -85,7 +86,7 @@ export function flexTreeFromCursor( cursor: ITreeCursorSynchronous, ): UnhydratedFlexTreeNode { assert(cursor.mode === CursorLocationType.Nodes, "Expected nodes cursor"); - const schema = context.schema.get(cursor.type) ?? fail("missing schema"); + const schema = context.schema.get(cursor.type) ?? unknownTypeError(cursor.type); const storedSchema = getStoredSchema( schema as SimpleNodeSchemaBase as SimpleNodeSchema, ); diff --git a/packages/dds/tree/src/simple-tree/api/customTree.ts b/packages/dds/tree/src/simple-tree/api/customTree.ts index 3d3b44e70fb1..14c7590f5929 100644 --- a/packages/dds/tree/src/simple-tree/api/customTree.ts +++ b/packages/dds/tree/src/simple-tree/api/customTree.ts @@ -6,6 +6,7 @@ import type { IFluidHandle } from "@fluidframework/core-interfaces"; import { assert, fail } from "@fluidframework/core-utils/internal"; import { isFluidHandle } from "@fluidframework/runtime-utils/internal"; +import { UsageError } from "@fluidframework/telemetry-utils/internal"; import { EmptyKey, @@ -231,3 +232,9 @@ export function replaceHandles(tree: unknown, replacer: HandleConverter): } }); } + +export function unknownTypeError(type: string): never { + throw new UsageError( + `Failed to parse tree due to occurrence of type ${JSON.stringify(type)} which is not defined in this context.`, + ); +} diff --git a/packages/dds/tree/src/simple-tree/api/verboseTree.ts b/packages/dds/tree/src/simple-tree/api/verboseTree.ts index 8dd4d4d1fb96..0766da8ba746 100644 --- a/packages/dds/tree/src/simple-tree/api/verboseTree.ts +++ b/packages/dds/tree/src/simple-tree/api/verboseTree.ts @@ -6,6 +6,7 @@ import type { IFluidHandle } from "@fluidframework/core-interfaces"; import { isFluidHandle } from "@fluidframework/runtime-utils/internal"; import { assert, fail } from "@fluidframework/core-utils/internal"; +import { UsageError } from "@fluidframework/telemetry-utils/internal"; import { aboveRootPlaceholder, @@ -14,7 +15,6 @@ import { type FieldKey, type ITreeCursor, type ITreeCursorSynchronous, - type TreeNodeSchemaIdentifier, } from "../../core/index.js"; import { brand } from "../../util/index.js"; import type { @@ -40,6 +40,7 @@ import { isObjectNodeSchema } from "../node-kinds/index.js"; import { customFromCursor, replaceHandles, + unknownTypeError, type CustomTreeNode, type HandleConverter, type SchemalessParseOptions, @@ -153,12 +154,14 @@ export function applySchemaToParserOptions( return key; }, parse: (type, inputKey): FieldKey => { - const simpleNodeSchema = - context.schema.get(brand(type)) ?? fail(0xb3a /* missing schema */); + const simpleNodeSchema = context.schema.get(brand(type)) ?? unknownTypeError(type); if (isObjectNodeSchema(simpleNodeSchema)) { - const info = - simpleNodeSchema.flexKeyMap.get(inputKey) ?? - fail(0xb3b /* missing field info */); + const info = simpleNodeSchema.flexKeyMap.get(inputKey); + if (info === undefined) { + throw new UsageError( + `Failed to parse VerboseTree due to unexpected key ${JSON.stringify(inputKey)} on type ${JSON.stringify(type)}.`, + ); + } return info.storedKey; } return brand(inputKey); @@ -203,19 +206,19 @@ function verboseTreeAdapter(options: SchemalessParseOptions): CursorAdapter { switch (typeof node) { case "number": - return numberSchema.identifier as TreeNodeSchemaIdentifier; + return brand(numberSchema.identifier); case "string": - return stringSchema.identifier as TreeNodeSchemaIdentifier; + return brand(stringSchema.identifier); case "boolean": - return booleanSchema.identifier as TreeNodeSchemaIdentifier; + return brand(booleanSchema.identifier); default: if (node === null) { - return nullSchema.identifier as TreeNodeSchemaIdentifier; + return brand(nullSchema.identifier); } if (isFluidHandle(node)) { - return handleSchema.identifier as TreeNodeSchemaIdentifier; + return brand(handleSchema.identifier); } - return node.type as TreeNodeSchemaIdentifier; + return brand(node.type); } }, keysFromNode: (node: VerboseTree): readonly FieldKey[] => { diff --git a/packages/dds/tree/src/test/simple-tree/api/integrationTests.spec.ts b/packages/dds/tree/src/test/simple-tree/api/integrationTests.spec.ts index 0e8e41e5582d..e48d126ed513 100644 --- a/packages/dds/tree/src/test/simple-tree/api/integrationTests.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/api/integrationTests.spec.ts @@ -47,7 +47,7 @@ describe("simple-tree API integration tests", () => { () => { array.insertAtEnd(obj); }, - validateUsageError(/already in a tree/), + validateUsageError(/more than one place/), ); }); diff --git a/packages/dds/tree/src/test/simple-tree/api/treeNodeApi.spec.ts b/packages/dds/tree/src/test/simple-tree/api/treeNodeApi.spec.ts index 5b82d5ab8e04..a1e2876f7e44 100644 --- a/packages/dds/tree/src/test/simple-tree/api/treeNodeApi.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/api/treeNodeApi.spec.ts @@ -1751,6 +1751,47 @@ describe("treeNodeApi", () => { describe("verbose", () => { describe("importVerbose", () => { + it("unknown schema: leaf", () => { + // Input using schema not included in the context + assert.throws( + () => TreeAlpha.importVerbose(SchemaFactory.number, "x"), + validateUsageError( + /type "com.fluidframework.leaf.string" which is not defined in this context/, + ), + ); + }); + + it("unknown schema: non-leaf", () => { + const factory = new SchemaFactory("Test"); + class A extends factory.object("A", {}) {} + class B extends factory.object("B", {}) {} + // Input using schema not included in the context + assert.throws( + () => TreeAlpha.importVerbose(A, { type: B.identifier, fields: {} }), + validateUsageError(/type "Test.B" which is not defined/), + ); + }); + + it("invalid with known schema", () => { + const factory = new SchemaFactory("Test"); + class A extends factory.object("A", { a: SchemaFactory.string }) {} + assert.throws( + () => TreeAlpha.importVerbose(A, { type: A.identifier, fields: { wrong: "x" } }), + validateUsageError( + `Failed to parse VerboseTree due to unexpected key "wrong" on type "Test.A".`, + ), + ); + }); + + it("missing field with default", () => { + const factory = new SchemaFactory("Test"); + class A extends factory.object("A", { a: factory.identifier }) {} + assert.throws( + () => TreeAlpha.importVerbose(A, { type: A.identifier, fields: {} }), + validateUsageError(/Field_MissingRequiredChild/), + ); + }); + it("undefined", () => { // Valid assert.equal(TreeAlpha.importVerbose(schema.optional([]), undefined), undefined); @@ -1762,7 +1803,7 @@ describe("treeNodeApi", () => { // Undefined required, not provided assert.throws( () => TreeAlpha.importVerbose(schema.optional([]), 1), - validateUsageError(/does not conform to schema/), + validateUsageError(/Failed to parse tree/), ); }); @@ -1772,7 +1813,7 @@ describe("treeNodeApi", () => { // invalid assert.throws( () => TreeAlpha.importVerbose([schema.null, schema.number], "x"), - validateUsageError(/does not conform to schema/), + validateUsageError(/Failed to parse tree/), ); }); @@ -1821,10 +1862,14 @@ describe("treeNodeApi", () => { const testTree = new Point3D({ x: 1, y: 2, z: 3 }); const exported = TreeAlpha.exportVerbose(testTree); - // TODO:AB#26720 The error here should be more clear. + // TODO:AB#26720 The error here should be more clear: + // perhaps reference allowUnknownOptionalFields and stored keys specifically. assert.throws( () => TreeAlpha.importVerbose(Point2D, exported), - (error: Error) => validateAssertionError(error, /missing field info/), + + validateUsageError( + `Failed to parse VerboseTree due to unexpected key "z" on type "com.example.Point".`, + ), ); }); }); From 78bdc8f53b95c5be12bc0455412aea2c815e37f4 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Fri, 30 May 2025 18:54:00 -0700 Subject: [PATCH 08/25] Fix from merge --- packages/dds/tree/src/feature-libraries/mapTreeCursor.ts | 3 --- packages/dds/tree/src/simple-tree/prepareForInsertion.ts | 5 ++--- packages/dds/tree/src/simple-tree/toStoredSchema.ts | 2 +- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts b/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts index 47c6c42c8b1d..2ed30eb94219 100644 --- a/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts +++ b/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts @@ -13,20 +13,17 @@ import { type ITreeCursor, type MapTree, type NodeData, - type NodeData, aboveRootPlaceholder, detachedFieldAsKey, mapCursorField, rootField, rootFieldKey, - rootFieldKey, } from "../core/index.js"; import { type CursorAdapter, type CursorWithNode, type Field, - type Field, stackTreeFieldCursor, stackTreeNodeCursor, } from "./treeCursorUtils.js"; diff --git a/packages/dds/tree/src/simple-tree/prepareForInsertion.ts b/packages/dds/tree/src/simple-tree/prepareForInsertion.ts index a85e5f3ac820..de35e4064f1a 100644 --- a/packages/dds/tree/src/simple-tree/prepareForInsertion.ts +++ b/packages/dds/tree/src/simple-tree/prepareForInsertion.ts @@ -17,7 +17,6 @@ import { getSchemaAndPolicy, type FlexTreeHydratedContextMinimal, FieldKinds, - type FlexTreeHydratedContextMinimal, type FlexibleFieldContent, type FlexibleNodeContent, } from "../feature-libraries/index.js"; @@ -108,8 +107,8 @@ export function prepareForInsertionContextless = new Map< +export const convertFieldKind: ReadonlyMap = new Map< FieldKind, FlexFieldKind >([ From 6c94fef74a8bad5ada7b6e93e41f2996eeefad9a Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Fri, 30 May 2025 18:57:50 -0700 Subject: [PATCH 09/25] Fix test --- packages/dds/tree/src/test/simple-tree/api/create.spec.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/dds/tree/src/test/simple-tree/api/create.spec.ts b/packages/dds/tree/src/test/simple-tree/api/create.spec.ts index 81c77de90e51..408dbd7d4725 100644 --- a/packages/dds/tree/src/test/simple-tree/api/create.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/api/create.spec.ts @@ -13,7 +13,6 @@ import { import { SchemaFactory } from "../../../simple-tree/index.js"; import { validateUsageError } from "../../utils.js"; import { singleJsonCursor } from "../../json/index.js"; -import { validateAssertionError } from "@fluidframework/test-runtime-utils/internal"; describe("simple-tree create", () => { describe("createFromCursor", () => { @@ -26,7 +25,9 @@ describe("simple-tree create", () => { const cursor = singleJsonCursor("Hello world"); assert.throws( () => createFromCursor(SchemaFactory.number, cursor), - (e: Error) => validateAssertionError(e, /missing schema/), + validateUsageError( + `Failed to parse tree due to occurrence of type "com.fluidframework.leaf.string" which is not defined in this context.`, + ), ); }); From 97aa5d77fe554b110162a585e2a3b33a1e1407f6 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Fri, 30 May 2025 21:46:41 -0700 Subject: [PATCH 10/25] Fix duplicate events --- packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index f3b59043782b..ad5d8319cc33 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -397,7 +397,6 @@ export class UnhydratedFlexTreeField } this.lazyChildren = edit(this.children) ?? this.children; - this.parent?.emitChangedEvent(this.key); // Set parents for all new map trees. for (const [index, tree] of this.children.entries()) { From 234ff663f465059373dec77e10cb97ba596839d1 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Fri, 30 May 2025 21:46:56 -0700 Subject: [PATCH 11/25] Fix a few tests --- .../core/unhydratedFlexTree.spec.ts | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts b/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts index c96ffc1f16b5..5f40830c1c0b 100644 --- a/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts @@ -7,15 +7,16 @@ import { strict as assert } from "node:assert"; import { cursorForMapTreeNode, FieldKinds } from "../../../feature-libraries/index.js"; import { - deepCopyMapTree, EmptyKey, type ExclusiveMapTree, type FieldKey, + type MapTree, type Value, } from "../../../core/index.js"; import { brand } from "../../../util/index.js"; -import type { - UnhydratedOptionalField, +import { + UnhydratedFlexTreeNode, + type UnhydratedOptionalField, // eslint-disable-next-line import/no-internal-modules } from "../../../simple-tree/core/unhydratedFlexTree.js"; import { SchemaFactory, stringSchema } from "../../../simple-tree/index.js"; @@ -76,9 +77,13 @@ describe("unhydratedFlexTree", () => { arrayNodeSchemaSimple, objectSchemaSimple, ]); - const map = flexTreeFromCursor(context, cursorForMapTreeNode(mapMapTree)); - const arrayNode = flexTreeFromCursor(context, cursorForMapTreeNode(fieldNodeMapTree)); + const object = flexTreeFromCursor(context, cursorForMapTreeNode(objectMapTree)); + const map = object.getBoxed(objectMapKey).boxedAt(0) ?? assert.fail(); + const arrayNode = object.getBoxed(objectFieldNodeKey).boxedAt(0) ?? assert.fail(); + + assert(map instanceof UnhydratedFlexTreeNode); + assert(arrayNode instanceof UnhydratedFlexTreeNode); it("can get their type", () => { assert.equal(map.type, "Test.Map"); @@ -212,27 +217,26 @@ describe("unhydratedFlexTree", () => { describe("can mutate", () => { it("required fields", () => { - const mutableObjectMapTree = deepCopyMapTree(objectMapTree); - const mutableObjectMapTreeMap = mutableObjectMapTree.fields.get(objectMapKey)?.[0]; - assert(mutableObjectMapTreeMap !== undefined); - const mutableObject = flexTreeFromCursor( - context, - cursorForMapTreeNode(mutableObjectMapTree), - ); + const mutableObject = flexTreeFromCursor(context, cursorForMapTreeNode(objectMapTree)); + // Find a field to edit. In this case the one with the map in it. const field = mutableObject.getBoxed(objectMapKey) as UnhydratedOptionalField; const oldMap = field.boxedAt(0); - assert(oldMap !== undefined); + assert(oldMap instanceof UnhydratedFlexTreeNode); assert.equal(oldMap.parentField.parent.parent, mutableObject); + + // Allocate a new node const newMap = flexTreeFromCursor(context, cursorForMapTreeNode(mapMapTree)); assert.notEqual(newMap, oldMap); assert.equal(newMap.parentField.parent.parent, undefined); + // Replace the old map with a new map field.editor.set(newMap); assert.equal(oldMap.parentField.parent.parent, undefined); assert.equal(newMap.parentField.parent.parent, mutableObject); assert.equal(field.boxedAt(0), newMap); + // Replace the new map with the old map again - field.editor.set(mutableObjectMapTreeMap); + field.editor.set(oldMap); assert.equal(oldMap.parentField.parent.parent, mutableObject); assert.equal(newMap.parentField.parent.parent, undefined); assert.equal(field.boxedAt(0), oldMap); @@ -243,7 +247,8 @@ describe("unhydratedFlexTree", () => { const field = mutableMap.getBoxed(mapKey) as UnhydratedOptionalField; const oldValue = field.boxedAt(0); const newValue = `new ${childValue}`; - field.editor.set({ ...mapChildMapTree, value: newValue }); + const newTree: MapTree = { ...mapChildMapTree, value: newValue }; + field.editor.set(flexTreeFromCursor(context, cursorForMapTreeNode(newTree))); assert.equal(field.boxedAt(0)?.value, newValue); assert.notEqual(newValue, oldValue); field.editor.set(undefined); From 787f5c09c6268e6be837b0307ae33ec94ee0e861 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Fri, 30 May 2025 21:53:20 -0700 Subject: [PATCH 12/25] Fix remaining test errors --- .../simple-tree/core/unhydratedFlexTree.spec.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts b/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts index 5f40830c1c0b..6ea57549e288 100644 --- a/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts @@ -265,13 +265,17 @@ describe("unhydratedFlexTree", () => { assert(field.is(FieldKinds.sequence)); const values = (): Value[] => Array.from(field.boxedIterator(), (n): Value => n.value); assert.deepEqual(values(), [childValue]); + const treeC: MapTree = { ...mapChildMapTree, value: "c" }; + const treeD: MapTree = { ...mapChildMapTree, value: "d" }; field.editor.insert(1, [ - { ...mapChildMapTree, value: "c" }, - { ...mapChildMapTree, value: "d" }, + flexTreeFromCursor(context, cursorForMapTreeNode(treeC)), + flexTreeFromCursor(context, cursorForMapTreeNode(treeD)), ]); + const treeA: MapTree = { ...mapChildMapTree, value: "a" }; + const treeB: MapTree = { ...mapChildMapTree, value: "b" }; field.editor.insert(0, [ - { ...mapChildMapTree, value: "a" }, - { ...mapChildMapTree, value: "b" }, + flexTreeFromCursor(context, cursorForMapTreeNode(treeA)), + flexTreeFromCursor(context, cursorForMapTreeNode(treeB)), ]); assert.deepEqual(values(), ["a", "b", childValue, "c", "d"]); field.editor.remove(2, 1); @@ -289,9 +293,10 @@ describe("unhydratedFlexTree", () => { ); const field = mutableFieldNode.getBoxed(EmptyKey); assert(field.is(FieldKinds.sequence)); - const newContent: ExclusiveMapTree[] = []; + const newContent: UnhydratedFlexTreeNode[] = []; for (let i = 0; i < 10000; i++) { - newContent.push({ ...mapChildMapTree, value: String(i) }); + const tree: MapTree = { ...mapChildMapTree, value: String(i) }; + newContent.push(flexTreeFromCursor(context, cursorForMapTreeNode(tree))); } field.editor.insert(0, newContent); assert.equal(field.length, newContent.length); From b756abe1209e3b9f1a281a8a0b9f4fcb953aabed Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Mon, 2 Jun 2025 09:23:01 -0700 Subject: [PATCH 13/25] Add changeset --- .changeset/fruity-laws-rule.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/fruity-laws-rule.md diff --git a/.changeset/fruity-laws-rule.md b/.changeset/fruity-laws-rule.md new file mode 100644 index 000000000000..a66c551d722d --- /dev/null +++ b/.changeset/fruity-laws-rule.md @@ -0,0 +1,9 @@ +--- +"@fluidframework/tree": minor +"fluid-framework": minor +"__section": tree +--- +Defaulted identifier fields on unhydrated nodes are now enumerable + +Previously, there was a special case for defaulted [identifier](https://fluidframework.com/docs/api/fluid-framework/schemafactory-class#identifier-property) fields on unhydrated nodes where they were not enumerable. +This special case has been removed: they are now enumerable independent of hydration status and defaulting. From f11ccfe8f78a062224182c7e1d08993f1c88362a Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Mon, 2 Jun 2025 09:53:34 -0700 Subject: [PATCH 14/25] cleanup imports --- packages/dds/tree/src/feature-libraries/index.ts | 2 ++ packages/dds/tree/src/shared-tree/treeAlpha.ts | 3 +-- packages/dds/tree/src/simple-tree/api/create.ts | 3 +-- packages/dds/tree/src/simple-tree/core/index.ts | 1 + .../dds/tree/src/simple-tree/core/unhydratedFlexTree.ts | 9 +++------ packages/dds/tree/src/simple-tree/index.ts | 1 + packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts | 7 +++---- 7 files changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/index.ts b/packages/dds/tree/src/feature-libraries/index.ts index 93807d9c19f8..966d7c6db4b4 100644 --- a/packages/dds/tree/src/feature-libraries/index.ts +++ b/packages/dds/tree/src/feature-libraries/index.ts @@ -18,6 +18,8 @@ export { type MinimalMapTreeNodeView, mapTreeFieldsWithField, mapTreeWithField, + type MapTreeFieldViewGeneric, + type MapTreeNodeViewGeneric, } from "./mapTreeCursor.js"; export { buildForest } from "./object-forest/index.js"; export { diff --git a/packages/dds/tree/src/shared-tree/treeAlpha.ts b/packages/dds/tree/src/shared-tree/treeAlpha.ts index 54bfa15871f0..29bf5ba4b460 100644 --- a/packages/dds/tree/src/shared-tree/treeAlpha.ts +++ b/packages/dds/tree/src/shared-tree/treeAlpha.ts @@ -39,6 +39,7 @@ import { treeNodeApi, getIdentifierFromNode, mapTreeFromNodeData, + getOrCreateNodeFromInnerNode, } from "../simple-tree/index.js"; import { extractFromOpaque, type JsonCompatible } from "../util/index.js"; import type { CodecWriteOptions, ICodecOptions } from "../codec/index.js"; @@ -58,8 +59,6 @@ import { import { independentInitializedView, type ViewContent } from "./independentView.js"; import { SchematizingSimpleTreeView, ViewSlot } from "./schematizingTreeView.js"; import { currentVersion, noopValidator } from "../codec/index.js"; -// eslint-disable-next-line import/no-internal-modules -import { getOrCreateNodeFromInnerNode } from "../simple-tree/core/index.js"; const identifier: TreeIdentifierUtils = (node: TreeNode): string | undefined => { const nodeIdentifier = getIdentifierFromNode(node, "uncompressed"); diff --git a/packages/dds/tree/src/simple-tree/api/create.ts b/packages/dds/tree/src/simple-tree/api/create.ts index bf1ac50f4cf8..afcbfd950ee5 100644 --- a/packages/dds/tree/src/simple-tree/api/create.ts +++ b/packages/dds/tree/src/simple-tree/api/create.ts @@ -19,6 +19,7 @@ import { type NodeKind, type Unhydrated, UnhydratedFlexTreeNode, + createField, } from "../core/index.js"; import { defaultSchemaPolicy, @@ -28,8 +29,6 @@ import { import { getUnhydratedContext } from "../createContext.js"; import { createUnknownOptionalFieldPolicy } from "../node-kinds/index.js"; import type { SimpleNodeSchema, SimpleNodeSchemaBase } from "../simpleSchema.js"; -// eslint-disable-next-line import/no-internal-modules -import { createField } from "../core/unhydratedFlexTree.js"; import { getStoredSchema } from "../toStoredSchema.js"; import { unknownTypeError } from "./customTree.js"; diff --git a/packages/dds/tree/src/simple-tree/core/index.ts b/packages/dds/tree/src/simple-tree/core/index.ts index 6530755184a4..1473019af580 100644 --- a/packages/dds/tree/src/simple-tree/core/index.ts +++ b/packages/dds/tree/src/simple-tree/core/index.ts @@ -40,4 +40,5 @@ export { UnhydratedFlexTreeNode, UnhydratedSequenceField, UnhydratedContext, + createField, } from "./unhydratedFlexTree.js"; diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index ad5d8319cc33..dbb7a6834210 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -41,22 +41,19 @@ import { type FlexFieldKind, FieldKinds, type SequenceFieldEditBuilder, + cursorForMapTreeNode, type OptionalFieldEditBuilder, type ValueFieldEditBuilder, type FlexibleNodeContent, type FlexTreeHydratedContextMinimal, - cursorForMapTreeNode, type FlexibleFieldContent, + type MapTreeFieldViewGeneric, + type MapTreeNodeViewGeneric, } from "../../feature-libraries/index.js"; import { brand, getOrCreate } from "../../util/index.js"; import type { Context } from "./context.js"; import type { ContextualFieldProvider } from "../schemaTypes.js"; -import type { - MapTreeFieldViewGeneric, - MapTreeNodeViewGeneric, - // eslint-disable-next-line import/no-internal-modules -} from "../../feature-libraries/mapTreeCursor.js"; interface UnhydratedTreeSequenceFieldEditBuilder extends SequenceFieldEditBuilder {} diff --git a/packages/dds/tree/src/simple-tree/index.ts b/packages/dds/tree/src/simple-tree/index.ts index b7c32c83b192..ed39c723d207 100644 --- a/packages/dds/tree/src/simple-tree/index.ts +++ b/packages/dds/tree/src/simple-tree/index.ts @@ -22,6 +22,7 @@ export { HydratedContext, SimpleContextSlot, getOrCreateInnerNode, + getOrCreateNodeFromInnerNode, getKernel, } from "./core/index.js"; export { diff --git a/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts b/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts index 153a8b8115f3..98ecbc5fc1aa 100644 --- a/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts @@ -21,6 +21,7 @@ import { stringSchema, type TreeNodeSchema, type ValidateRecursiveSchema, + getKernel, } from "../../simple-tree/index.js"; import { createFieldSchema, @@ -41,14 +42,12 @@ import { } from "../../feature-libraries/index.js"; import { validateUsageError } from "../utils.js"; // eslint-disable-next-line import/no-internal-modules -import { UnhydratedFlexTreeNode } from "../../simple-tree/core/unhydratedFlexTree.js"; +import { UnhydratedFlexTreeNode } from "../../simple-tree/core/index.js"; // eslint-disable-next-line import/no-internal-modules import { getUnhydratedContext } from "../../simple-tree/createContext.js"; // eslint-disable-next-line import/no-internal-modules -import { getKernel } from "../../simple-tree/core/index.js"; -import { hydrate } from "./utils.js"; -// eslint-disable-next-line import/no-internal-modules import { prepareContentForHydration } from "../../simple-tree/prepareForInsertion.js"; +import { hydrate } from "./utils.js"; describe("toMapTree", () => { it("string", () => { From 0b364b0b665dc90fac898619d7e86eaeddf50924 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Mon, 2 Jun 2025 10:07:49 -0700 Subject: [PATCH 15/25] Replace replace unhydratedFlexTreeNodeToTreeNode with treeNode property --- .../src/simple-tree/core/getOrCreateNode.ts | 3 +-- .../dds/tree/src/simple-tree/core/index.ts | 1 - .../src/simple-tree/core/treeNodeKernel.ts | 22 +++---------------- .../simple-tree/core/unhydratedFlexTree.ts | 6 +++++ .../src/simple-tree/prepareForInsertion.ts | 9 ++------ 5 files changed, 12 insertions(+), 29 deletions(-) diff --git a/packages/dds/tree/src/simple-tree/core/getOrCreateNode.ts b/packages/dds/tree/src/simple-tree/core/getOrCreateNode.ts index ef752fe51e98..7aaf0d8fd615 100644 --- a/packages/dds/tree/src/simple-tree/core/getOrCreateNode.ts +++ b/packages/dds/tree/src/simple-tree/core/getOrCreateNode.ts @@ -8,7 +8,6 @@ import type { TreeValue } from "../../core/index.js"; import type { TreeNode } from "./treeNode.js"; import { type InnerNode, - unhydratedFlexTreeNodeToTreeNode, simpleTreeNodeSlot, createTreeNodeFromInner, } from "./treeNodeKernel.js"; @@ -23,7 +22,7 @@ import { UnhydratedFlexTreeNode } from "./unhydratedFlexTree.js"; export function getOrCreateNodeFromInnerNode(flexNode: InnerNode): TreeNode | TreeValue { const cached = flexNode instanceof UnhydratedFlexTreeNode - ? unhydratedFlexTreeNodeToTreeNode.get(flexNode) + ? flexNode.treeNode : flexNode.anchorNode.slots.get(simpleTreeNodeSlot); if (cached !== undefined) { diff --git a/packages/dds/tree/src/simple-tree/core/index.ts b/packages/dds/tree/src/simple-tree/core/index.ts index 1473019af580..312e9ab882eb 100644 --- a/packages/dds/tree/src/simple-tree/core/index.ts +++ b/packages/dds/tree/src/simple-tree/core/index.ts @@ -10,7 +10,6 @@ export { tryGetTreeNodeSchema, type InnerNode, tryDisposeTreeNode, - unhydratedFlexTreeNodeToTreeNode, getOrCreateInnerNode, treeNodeFromAnchor, getSimpleNodeSchemaFromInnerNode, diff --git a/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts b/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts index b9ae2fbe23df..e3e90a048fe2 100644 --- a/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts +++ b/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts @@ -148,7 +148,8 @@ export class TreeNodeKernel { if (innerNode instanceof UnhydratedFlexTreeNode) { // Unhydrated case - unhydratedFlexTreeNodeToTreeNodeInternal.set(innerNode, node); + debugAssert(() => innerNode.treeNode === undefined); + innerNode.treeNode = node; // Register for change events from the unhydrated flex node. // These will be fired if the unhydrated node is edited, and will also be forwarded later to the hydrated node. this.#hydrationState = { @@ -160,7 +161,7 @@ export class TreeNodeKernel { let unhydratedNode: UnhydratedFlexTreeNode | undefined = innerNode; while (unhydratedNode !== undefined) { - const treeNode = unhydratedFlexTreeNodeToTreeNodeInternal.get(unhydratedNode); + const treeNode = unhydratedNode.treeNode; if (treeNode !== undefined) { const kernel = getKernel(treeNode); kernel.#unhydratedEvents.value.emit("subtreeChangedAfterBatch"); @@ -203,7 +204,6 @@ export class TreeNodeKernel { public hydrate(anchors: AnchorSet, path: UpPath): void { assert(!this.disposed, 0xa2a /* cannot hydrate a disposed node */); assert(!isHydrated(this.#hydrationState), 0xa2b /* hydration should only happen once */); - unhydratedFlexTreeNodeToTreeNodeInternal.delete(this.#hydrationState.innerNode); const anchor = anchors.track(path); const anchorNode = @@ -375,22 +375,6 @@ type KernelEvents = Pick; */ export type InnerNode = FlexTreeNode | UnhydratedFlexTreeNode; -/** - * Associates a given {@link UnhydratedFlexTreeNode} with a {@link TreeNode}. - */ -const unhydratedFlexTreeNodeToTreeNodeInternal = new WeakMap< - UnhydratedFlexTreeNode, - TreeNode ->(); -/** - * Retrieves the {@link TreeNode} associated with the given {@link UnhydratedFlexTreeNode} if any. - */ -export const unhydratedFlexTreeNodeToTreeNode = - unhydratedFlexTreeNodeToTreeNodeInternal as Pick< - WeakMap, - "get" - >; - /** * An anchor slot which associates an anchor with its corresponding {@link TreeNode}, if there is one. * @remarks diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index dbb7a6834210..67749dc8f5df 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -54,6 +54,7 @@ import { brand, getOrCreate } from "../../util/index.js"; import type { Context } from "./context.js"; import type { ContextualFieldProvider } from "../schemaTypes.js"; +import type { TreeNode } from "./treeNode.js"; interface UnhydratedTreeSequenceFieldEditBuilder extends SequenceFieldEditBuilder {} @@ -85,6 +86,11 @@ export class UnhydratedFlexTreeNode ); } + /** + * Cache storing the TreeNode for this inner node. + */ + public treeNode: TreeNode | undefined = undefined; + public readonly [flexTreeMarker] = FlexTreeEntityKind.Node as const; private readonly _events = createEmitter(); diff --git a/packages/dds/tree/src/simple-tree/prepareForInsertion.ts b/packages/dds/tree/src/simple-tree/prepareForInsertion.ts index de35e4064f1a..4036bdd597ca 100644 --- a/packages/dds/tree/src/simple-tree/prepareForInsertion.ts +++ b/packages/dds/tree/src/simple-tree/prepareForInsertion.ts @@ -28,12 +28,7 @@ import { import { type InsertableContent, mapTreeFromNodeData } from "./toMapTree.js"; import { UsageError } from "@fluidframework/telemetry-utils/internal"; import { brand } from "../util/index.js"; -import { - getKernel, - type TreeNode, - type UnhydratedFlexTreeNode, - unhydratedFlexTreeNodeToTreeNode, -} from "./core/index.js"; +import { getKernel, type TreeNode, type UnhydratedFlexTreeNode } from "./core/index.js"; import { debugAssert, oob } from "@fluidframework/core-utils/internal"; import { inSchemaOrThrow, isFieldInSchema } from "../feature-libraries/index.js"; import { convertField } from "./toStoredSchema.js"; @@ -231,7 +226,7 @@ function walkMapTree( for (let next: Next | undefined = [path, root]; next !== undefined; next = nexts.pop()) { const [p, node] = next; if (node !== undefined) { - const treeNode = unhydratedFlexTreeNodeToTreeNode.get(node); + const treeNode = node.treeNode; if (treeNode !== undefined) { onVisitTreeNode(p, treeNode); } From ea9a42e801ad1e5824504a6c84aa227c8a63c465 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Mon, 2 Jun 2025 11:13:07 -0700 Subject: [PATCH 16/25] simplify iteration --- .../default-schema/schemaChecker.ts | 3 ++- .../tree/src/feature-libraries/mapTreeCursor.ts | 4 ++-- .../src/simple-tree/core/unhydratedFlexTree.ts | 17 ++++++++--------- packages/dds/tree/src/test/util/utils.spec.ts | 7 +++++++ packages/dds/tree/src/util/index.ts | 1 + packages/dds/tree/src/util/utils.ts | 7 +++++++ 6 files changed, 27 insertions(+), 12 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts b/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts index a5f99923d953..4a4472781f9d 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts @@ -16,6 +16,7 @@ import { } from "../../core/index.js"; import { allowsValue } from "../valueUtilities.js"; import type { MapTreeFieldViewGeneric, MinimalMapTreeNodeView } from "../mapTreeCursor.js"; +import { iterableHasSome } from "../../util/index.js"; export enum SchemaValidationError { Field_KindNotInSchemaPolicy, @@ -57,7 +58,7 @@ export function isNodeInSchema( // Validate the node is well formed according to its schema if (schema instanceof LeafNodeStoredSchema) { - if (node.fields.size !== 0) { + if (iterableHasSome(node.fields)) { return SchemaValidationError.LeafNode_FieldsNotAllowed; } if (!allowsValue(schema.leafValue, node.value)) { diff --git a/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts b/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts index 2ed30eb94219..31ddc514a0fd 100644 --- a/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts +++ b/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts @@ -40,11 +40,11 @@ export interface MapTreeNodeViewGeneric extends NodeData { * The non-empty fields on this node. * @remarks * This is the subset of map needed to view the tree. - * Theoretically "size" could be removed by measuring the length of keys, but it is included for convenience. + * Theoretically `Symbol.iterator` and "keys" are redundant. */ readonly fields: Pick< ReadonlyMap>, - typeof Symbol.iterator | "get" | "size" | "keys" + typeof Symbol.iterator | "get" | "keys" >; } diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index 67749dc8f5df..b235af4731f8 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -122,9 +122,13 @@ export class UnhydratedFlexTreeNode /** * The non-empty fields on this node. + * @remarks + * This is needed to implement {@link MapTreeNodeViewGeneric.fields}, which must omit empty fields. + * Due to having to detect if a field is empty, this forces the evaluation of any pending defaults in the fields. + * Use {@link allFieldsLazy} to avoid evaluating pending defaults. */ public get fields(): ReadonlyMap { - // TODO: doing this copy on every access is inefficient. SOme caching+invalidation or removal of the need for a separate map would be better. + // TODO: doing this copy on every access is inefficient. Some caching+invalidation or removal of the need for a separate map would be better. const entries = [...this.fieldsAll.entries()].filter(([, field]) => field.length > 0); return new Map(entries); } @@ -207,7 +211,7 @@ export class UnhydratedFlexTreeNode } public tryGetField(key: FieldKey): UnhydratedFlexTreeField | undefined { - const field = this.fields.get(key); + const field = this.fieldsAll.get(key); // Only return the field if it is not empty, in order to fulfill the contract of `tryGetField`. if (field !== undefined && field.length > 0) { return field; @@ -220,16 +224,11 @@ export class UnhydratedFlexTreeNode } public boxedIterator(): IterableIterator { - return [...this.fields.values()] - .filter((field) => field.children.length > 0) - [Symbol.iterator](); + return Array.from(this.fields, ([key, field]) => field)[Symbol.iterator](); } public keys(): IterableIterator { - return [...this.fields.entries()] - .filter((kv) => kv[1].children.length > 0) - .map((kv) => kv[0]) - [Symbol.iterator](); + return Array.from(this.fields, ([key]) => key)[Symbol.iterator](); } public get value(): Value { diff --git a/packages/dds/tree/src/test/util/utils.spec.ts b/packages/dds/tree/src/test/util/utils.spec.ts index cdd2e6aa12c4..bd1b48560cd0 100644 --- a/packages/dds/tree/src/test/util/utils.spec.ts +++ b/packages/dds/tree/src/test/util/utils.spec.ts @@ -10,6 +10,7 @@ import { capitalize, copyProperty, defineLazyCachedProperty, + iterableHasSome, mapIterable, transformObjectMap, } from "../../util/index.js"; @@ -157,4 +158,10 @@ describe("Utils", () => { assert.equal(delegateCallCount, 6); }); }); + + it("iterableHasSome", () => { + assert(!iterableHasSome([])); + assert(iterableHasSome([1])); + assert(!iterableHasSome(new Map([]))); + }); }); diff --git a/packages/dds/tree/src/util/index.ts b/packages/dds/tree/src/util/index.ts index e4a390467f43..fc6b2995fc47 100644 --- a/packages/dds/tree/src/util/index.ts +++ b/packages/dds/tree/src/util/index.ts @@ -95,6 +95,7 @@ export { defineLazyCachedProperty, copyPropertyIfDefined as copyProperty, getOrAddInMap, + iterableHasSome, } from "./utils.js"; export { ReferenceCountedBase, type ReferenceCounted } from "./referenceCounting.js"; diff --git a/packages/dds/tree/src/util/utils.ts b/packages/dds/tree/src/util/utils.ts index 97b5c89c92b2..5dc107c53bd5 100644 --- a/packages/dds/tree/src/util/utils.ts +++ b/packages/dds/tree/src/util/utils.ts @@ -98,6 +98,13 @@ export function hasSome(array: readonly T[]): array is [T, ...T[]] { return array.length > 0; } +/** + * Returns true if and only if the given iterable has at least one element. + */ +export function iterableHasSome(iterable: Iterable): boolean { + return iterable[Symbol.iterator]().next().done === false; +} + /** * Returns true if and only if the given array has exactly one element. * @param array - The array to check. From 02f924575f672e64cd2df30d3e1fe9cae5e62fb3 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Mon, 2 Jun 2025 11:38:22 -0700 Subject: [PATCH 17/25] Avoid copy in UnhydratedFlexTreeNode --- .../default-schema/schemaChecker.ts | 4 ++-- .../tree/src/feature-libraries/mapTreeCursor.ts | 6 +++--- .../src/simple-tree/core/unhydratedFlexTree.ts | 15 +++++++++------ 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts b/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts index 4a4472781f9d..d7b826afacf0 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts @@ -16,7 +16,7 @@ import { } from "../../core/index.js"; import { allowsValue } from "../valueUtilities.js"; import type { MapTreeFieldViewGeneric, MinimalMapTreeNodeView } from "../mapTreeCursor.js"; -import { iterableHasSome } from "../../util/index.js"; +import { iterableHasSome, mapIterable } from "../../util/index.js"; export enum SchemaValidationError { Field_KindNotInSchemaPolicy, @@ -70,7 +70,7 @@ export function isNodeInSchema( } if (schema instanceof ObjectNodeStoredSchema) { - const uncheckedFieldsFromNode = new Set(node.fields.keys()); + const uncheckedFieldsFromNode = new Set(mapIterable(node.fields, ([key, field]) => key)); for (const [fieldKey, fieldSchema] of schema.objectNodeFields) { const nodeField = node.fields.get(fieldKey) ?? []; const fieldInSchemaResult = isFieldInSchema(nodeField, fieldSchema, schemaAndPolicy); diff --git a/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts b/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts index 31ddc514a0fd..d1bcb5210ce8 100644 --- a/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts +++ b/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts @@ -43,8 +43,8 @@ export interface MapTreeNodeViewGeneric extends NodeData { * Theoretically `Symbol.iterator` and "keys" are redundant. */ readonly fields: Pick< - ReadonlyMap>, - typeof Symbol.iterator | "get" | "keys" + Map>, + typeof Symbol.iterator | "get" >; } @@ -127,7 +127,7 @@ export function cursorForMapTreeField>( const adapter: CursorAdapter = { value: (node) => node.value, type: (node) => node.type, - keysFromNode: (node) => [...node.fields.keys()], // TODO: don't convert this to array here. + keysFromNode: (node) => Array.from(node.fields, ([key, field]) => key), getFieldFromNode: (node, key): Field => { const field = node.fields.get(key) as | MinimalMapTreeNodeView[] diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index b235af4731f8..4d227d2e7000 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -50,7 +50,7 @@ import { type MapTreeFieldViewGeneric, type MapTreeNodeViewGeneric, } from "../../feature-libraries/index.js"; -import { brand, getOrCreate } from "../../util/index.js"; +import { brand, filterIterable, getOrCreate } from "../../util/index.js"; import type { Context } from "./context.js"; import type { ContextualFieldProvider } from "../schemaTypes.js"; @@ -127,11 +127,14 @@ export class UnhydratedFlexTreeNode * Due to having to detect if a field is empty, this forces the evaluation of any pending defaults in the fields. * Use {@link allFieldsLazy} to avoid evaluating pending defaults. */ - public get fields(): ReadonlyMap { - // TODO: doing this copy on every access is inefficient. Some caching+invalidation or removal of the need for a separate map would be better. - const entries = [...this.fieldsAll.entries()].filter(([, field]) => field.length > 0); - return new Map(entries); - } + public readonly fields: Pick< + Map, + typeof Symbol.iterator | "get" + > = { + get: (key: FieldKey): UnhydratedFlexTreeField | undefined => this.tryGetField(key), + [Symbol.iterator]: (): IterableIterator<[FieldKey, UnhydratedFlexTreeField]> => + filterIterable(this.fieldsAll, ([, field]) => field.length > 0), + }; /** * Gets all fields, without filtering out empty ones. From 07a47f84a55255ba82caa16077cba4436f83df89 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Mon, 2 Jun 2025 13:43:19 -0700 Subject: [PATCH 18/25] Simplify cast --- .../dds/tree/src/simple-tree/node-kinds/object/objectNode.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/dds/tree/src/simple-tree/node-kinds/object/objectNode.ts b/packages/dds/tree/src/simple-tree/node-kinds/object/objectNode.ts index bec8aaeef860..014e91dcbe88 100644 --- a/packages/dds/tree/src/simple-tree/node-kinds/object/objectNode.ts +++ b/packages/dds/tree/src/simple-tree/node-kinds/object/objectNode.ts @@ -45,7 +45,6 @@ import { type InsertableTreeFieldFromImplicitField, type FieldSchema, normalizeFieldSchema, - type ImplicitAllowedTypes, FieldKind, type NodeSchemaMetadata, type FieldSchemaAlpha, @@ -454,7 +453,7 @@ export function objectSchema< instance: TreeNodeValid, input: T2, ): UnhydratedFlexTreeNode { - return mapTreeFromNodeData(input as object, this as unknown as ImplicitAllowedTypes); + return mapTreeFromNodeData(input as object, this as Output); } protected static override constructorCached: MostDerivedData | undefined = undefined; From db1306c1d043391adab6f62e61cc617e2c9798ca Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Mon, 2 Jun 2025 13:43:37 -0700 Subject: [PATCH 19/25] add some docs --- .../simple-tree/core/unhydratedFlexTree.ts | 21 ++++++++++++------- .../tree/src/simple-tree/toStoredSchema.ts | 3 +++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index 4d227d2e7000..9e6c69be1a9a 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -68,12 +68,7 @@ interface LocationInField { } /** - * An unhydrated implementation of {@link FlexTreeNode} which wraps a {@link MapTree}. - * @remarks - * MapTreeNodes are unconditionally cached - - * when retrieved via {@link getOrCreateNodeFromInnerNode}, the same {@link MapTree} object will always produce the same `UnhydratedFlexTreeNode` object. - * - * Create a `UnhydratedFlexTreeNode` by calling {@link getOrCreate}. + * The {@link Unhydrated} implementation of {@link FlexTreeNode}. */ export class UnhydratedFlexTreeNode implements FlexTreeNode, MapTreeNodeViewGeneric @@ -87,7 +82,10 @@ export class UnhydratedFlexTreeNode } /** - * Cache storing the TreeNode for this inner node. + * Cache storing the {@link TreeNode} for this inner node. + * @remarks + * When creating a `TreeNode` for this `UnhydratedFlexTreeNode`, cache the `TreeNode` in this property: see {@link TreeNodeKernel}. + * See {@link getOrCreateNodeFromInnerNode} how to get the `TreeNode`, even if not already created, regardless of hydration status. */ public treeNode: TreeNode | undefined = undefined; @@ -308,6 +306,9 @@ const unparentedLocation: LocationInField = { index: -1, }; +/** + * The {@link Unhydrated} implementation of {@link FlexTreeField}. + */ export class UnhydratedFlexTreeField implements FlexTreeField, MapTreeFieldViewGeneric { @@ -426,6 +427,9 @@ export class UnhydratedFlexTreeField } } +/** + * The {@link Unhydrated} implementation of {@link FlexTreeOptionalField}. + */ export class UnhydratedOptionalField extends UnhydratedFlexTreeField implements FlexTreeOptionalField @@ -471,6 +475,9 @@ class UnhydratedRequiredField } } +/** + * The {@link Unhydrated} implementation of {@link FlexTreeSequenceField}. + */ export class UnhydratedSequenceField extends UnhydratedFlexTreeField implements FlexTreeSequenceField diff --git a/packages/dds/tree/src/simple-tree/toStoredSchema.ts b/packages/dds/tree/src/simple-tree/toStoredSchema.ts index b42bd1af236c..d6819c934739 100644 --- a/packages/dds/tree/src/simple-tree/toStoredSchema.ts +++ b/packages/dds/tree/src/simple-tree/toStoredSchema.ts @@ -93,6 +93,9 @@ export function convertField(schema: SimpleFieldSchema): TreeFieldStoredSchema { return { kind, types }; } +/** + * A map that converts {@link FieldKind} to {@link FlexFieldKind}. + */ export const convertFieldKind: ReadonlyMap = new Map< FieldKind, FlexFieldKind From 8d7531f6291c323b0366063ee87c6e0031acbb6f Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Mon, 2 Jun 2025 13:51:35 -0700 Subject: [PATCH 20/25] More and updated docs --- .../simple-tree/core/unhydratedFlexTree.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index 9e6c69be1a9a..ddfbe7ebfde8 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -102,15 +102,24 @@ export class UnhydratedFlexTreeNode /** * Create a new UnhydratedFlexTreeNode. - * @param location - the parentage of this node, if it is being created underneath an existing node and field, or undefined if not - * @remarks This class (and its subclasses) should not be directly constructed outside of this module. - * Instead, use {@link getOrCreateNodeFromInnerNode} to create a UnhydratedFlexTreeNode from a {@link MapTree}. - * A `UnhydratedFlexTreeNode` may never be constructed more than once for the same {@link MapTree} object. - * Instead, it should always be acquired via {@link getOrCreateNodeFromInnerNode}. */ public constructor( + /** + * The {@link NodeData} for this node. + */ public readonly data: NodeData, + /** + * All {@link UnhydratedFlexTreeField} for this node that have been created so far. + * @remarks + * This includes all non-empty fields, but also any empty fields which have been previously requested. + */ private readonly fieldsAll: Map, + /** + * The {@link Context} for this node. + * @remarks + * Provides access to all schema reachable from this node. + * See {@link getUnhydratedContext}. + */ public readonly simpleContext: Context, ) { for (const [_key, field] of this.fieldsAll) { From e32016295f42ba3f58e4fe222c18a17012c8cb66 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Mon, 2 Jun 2025 15:31:18 -0700 Subject: [PATCH 21/25] SImplify and better document a few bits of unhydrated node logic --- .../dds/tree/src/simple-tree/api/create.ts | 6 +-- .../src/simple-tree/core/treeNodeKernel.ts | 18 +++----- .../simple-tree/core/unhydratedFlexTree.ts | 6 ++- .../dds/tree/src/simple-tree/toMapTree.ts | 43 +++++-------------- .../shared-tree/schematizingTreeView.spec.ts | 2 +- .../core/unhydratedFlexTree.spec.ts | 36 +++++++++------- .../test/simple-tree/unhydratedNode.spec.ts | 8 ++-- 7 files changed, 48 insertions(+), 71 deletions(-) diff --git a/packages/dds/tree/src/simple-tree/api/create.ts b/packages/dds/tree/src/simple-tree/api/create.ts index afcbfd950ee5..be6f34d153e1 100644 --- a/packages/dds/tree/src/simple-tree/api/create.ts +++ b/packages/dds/tree/src/simple-tree/api/create.ts @@ -43,7 +43,7 @@ export function createFromCursor( cursor: ITreeCursorSynchronous | undefined, ): Unhydrated> { const context = getUnhydratedContext(schema); - const mapTrees = cursor === undefined ? [] : [flexTreeFromCursor(context, cursor)]; + const mapTrees = cursor === undefined ? [] : [unhydratedFlexTreeFromCursor(context, cursor)]; const flexSchema = context.flexContext.schema; @@ -80,7 +80,7 @@ export function createFromCursor( * @remarks * This does not validate the node is in schema. */ -export function flexTreeFromCursor( +export function unhydratedFlexTreeFromCursor( context: Context, cursor: ITreeCursorSynchronous, ): UnhydratedFlexTreeNode { @@ -96,7 +96,7 @@ export function flexTreeFromCursor( context.flexContext, storedSchema.getFieldSchema(cursor.getFieldKey()).kind, cursor.getFieldKey(), - mapCursorField(cursor, () => flexTreeFromCursor(context, cursor)), + mapCursorField(cursor, () => unhydratedFlexTreeFromCursor(context, cursor)), ), ]), ); diff --git a/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts b/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts index e3e90a048fe2..b7616ad933a6 100644 --- a/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts +++ b/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts @@ -292,7 +292,7 @@ export class TreeNodeKernel { } /** - * Retrieves the flex node associated with the given target via {@link setInnerNode}. + * Retrieves the flex node associated with the given target. * @remarks * For {@link Unhydrated} nodes, this returns the MapTreeNode. * @@ -339,20 +339,12 @@ export class TreeNodeKernel { } /** - * Retrieves the InnerNode associated with the given target via {@link setInnerNode}, if any. - * @remarks - * If `target` is an unhydrated node, returns its UnhydratedFlexTreeNode. - * If `target` is a cooked node (or marinated but a FlexTreeNode exists) returns the FlexTreeNode. - * If the target is a marinated node with no FlexTreeNode for its anchor, returns undefined. + * Retrieves the {@link UnhydratedFlexTreeNode} if unhydrated. otherwise undefined. */ - public tryGetInnerNode(): InnerNode | undefined { + public getInnerNodeIfUnhydrated(): UnhydratedFlexTreeNode | undefined { if (isHydrated(this.#hydrationState)) { - return ( - this.#hydrationState.innerNode ?? - this.#hydrationState.anchorNode.slots.get(flexTreeSlot) - ); + return undefined; } - return this.#hydrationState.innerNode; } } @@ -418,7 +410,7 @@ export function getSimpleContextFromInnerNode(innerNode: InnerNode): Context { } /** - * Retrieves the flex node associated with the given target via {@link setInnerNode}. + * Retrieves the flex node associated with the given target. * @remarks * For {@link Unhydrated} nodes, this returns the MapTreeNode. * diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index ddfbe7ebfde8..2914f16c5ab6 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -84,10 +84,12 @@ export class UnhydratedFlexTreeNode /** * Cache storing the {@link TreeNode} for this inner node. * @remarks - * When creating a `TreeNode` for this `UnhydratedFlexTreeNode`, cache the `TreeNode` in this property: see {@link TreeNodeKernel}. + * When creating a `TreeNode` for this `UnhydratedFlexTreeNode`, cache the `TreeNode` in this property. + * Currently this is done by {@link TreeNodeKernel}. + * * See {@link getOrCreateNodeFromInnerNode} how to get the `TreeNode`, even if not already created, regardless of hydration status. */ - public treeNode: TreeNode | undefined = undefined; + public treeNode: TreeNode | undefined; public readonly [flexTreeMarker] = FlexTreeEntityKind.Node as const; diff --git a/packages/dds/tree/src/simple-tree/toMapTree.ts b/packages/dds/tree/src/simple-tree/toMapTree.ts index f70ba6a028a7..ba24717abb7a 100644 --- a/packages/dds/tree/src/simple-tree/toMapTree.ts +++ b/packages/dds/tree/src/simple-tree/toMapTree.ts @@ -31,10 +31,8 @@ import { } from "./schemaTypes.js"; import { getKernel, - getSimpleNodeSchemaFromInnerNode, isTreeNode, NodeKind, - type InnerNode, type TreeNode, type TreeNodeSchema, type Unhydrated, @@ -122,23 +120,20 @@ function nodeDataToMapTree( data: InsertableContent, allowedTypes: ReadonlySet, ): UnhydratedFlexTreeNode { - // A special cache path for processing unhydrated nodes. - // They already have the mapTree, so there is no need to recompute it. - const innerNode = tryGetInnerNode(data); - if (innerNode !== undefined) { - if (innerNode instanceof UnhydratedFlexTreeNode) { - if (!allowedTypes.has(getSimpleNodeSchemaFromInnerNode(innerNode))) { - throw new UsageError("Invalid schema for this context."); - } - return innerNode; - } else { + if (isTreeNode(data)) { + const kernel = getKernel(data); + const inner = kernel.getInnerNodeIfUnhydrated(); + if (inner === undefined) { // The node is already hydrated, meaning that it already got inserted into the tree previously throw new UsageError("A node may not be inserted into the tree more than once"); + } else { + if (!allowedTypes.has(kernel.schema)) { + throw new UsageError("Invalid schema for this context."); + } + return inner; } } - assert(!isTreeNode(data), 0xa23 /* data without an inner node cannot be TreeNode */); - const schema = getType(data, allowedTypes); let result: FlexContent; @@ -392,12 +387,7 @@ function objectToMapTree(data: FactoryContent, schema: TreeNodeSchema): FlexCont fieldInfo.schema.props?.defaultProvider ?? fail("missing field has no default provider"); const fieldProvider = extractFieldProvider(defaultProvider); - // eslint-disable-next-line unicorn/prefer-ternary - if (isConstant(fieldProvider)) { - children = fieldProvider(); - } else { - children = fieldProvider; - } + children = isConstant(fieldProvider) ? fieldProvider() : fieldProvider; } else { children = [nodeDataToMapTree(value, fieldInfo.schema.allowedTypeSet)]; } @@ -603,19 +593,6 @@ function allowsValue(schema: TreeNodeSchema, value: TreeValue): boolean { return false; } -/** - * Retrieves the InnerNode associated with the given target via {@link setInnerNode}, if any. - * @remarks - * If `target` is a unhydrated node, returns its MapTreeNode. - * If `target` is a cooked node (or marinated but a FlexTreeNode exists) returns the FlexTreeNode. - * If the target is not a node, or a marinated node with no FlexTreeNode for its anchor, returns undefined. - */ -function tryGetInnerNode(target: unknown): InnerNode | undefined { - if (isTreeNode(target)) { - return getKernel(target).tryGetInnerNode(); - } -} - /** * Content which can be used to build a node. * @remarks diff --git a/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts b/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts index 4983fccda241..f383e435db4f 100644 --- a/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts +++ b/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts @@ -119,7 +119,7 @@ describe("SchematizingSimpleTreeView", () => { const root = new Root({ content: 5 }); - const inner = getKernel(root).tryGetInnerNode() ?? assert.fail("Expected child"); + const inner = getKernel(root).getOrCreateInnerNode(); const field = inner.getBoxed(brand("content")); const child = field.boxedAt(0) ?? assert.fail("Expected child"); assert(child instanceof UnhydratedFlexTreeNode); diff --git a/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts b/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts index 6ea57549e288..e78d38f2ea4a 100644 --- a/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/core/unhydratedFlexTree.spec.ts @@ -23,7 +23,7 @@ import { SchemaFactory, stringSchema } from "../../../simple-tree/index.js"; // eslint-disable-next-line import/no-internal-modules import { getUnhydratedContext } from "../../../simple-tree/createContext.js"; // eslint-disable-next-line import/no-internal-modules -import { flexTreeFromCursor } from "../../../simple-tree/api/create.js"; +import { unhydratedFlexTreeFromCursor } from "../../../simple-tree/api/create.js"; import { expectEqualCursors } from "../../utils.js"; describe("unhydratedFlexTree", () => { @@ -78,7 +78,7 @@ describe("unhydratedFlexTree", () => { objectSchemaSimple, ]); - const object = flexTreeFromCursor(context, cursorForMapTreeNode(objectMapTree)); + const object = unhydratedFlexTreeFromCursor(context, cursorForMapTreeNode(objectMapTree)); const map = object.getBoxed(objectMapKey).boxedAt(0) ?? assert.fail(); const arrayNode = object.getBoxed(objectFieldNodeKey).boxedAt(0) ?? assert.fail(); @@ -164,7 +164,7 @@ describe("unhydratedFlexTree", () => { it("cannot be multiparented", () => { assert.throws(() => - flexTreeFromCursor( + unhydratedFlexTreeFromCursor( context, cursorForMapTreeNode({ type: brand("Parent of a node that already has another parent"), @@ -179,7 +179,7 @@ describe("unhydratedFlexTree", () => { fields: new Map(), }; assert.throws(() => { - flexTreeFromCursor( + unhydratedFlexTreeFromCursor( context, cursorForMapTreeNode({ type: brand("Parent with the same child twice in the same field"), @@ -217,7 +217,10 @@ describe("unhydratedFlexTree", () => { describe("can mutate", () => { it("required fields", () => { - const mutableObject = flexTreeFromCursor(context, cursorForMapTreeNode(objectMapTree)); + const mutableObject = unhydratedFlexTreeFromCursor( + context, + cursorForMapTreeNode(objectMapTree), + ); // Find a field to edit. In this case the one with the map in it. const field = mutableObject.getBoxed(objectMapKey) as UnhydratedOptionalField; const oldMap = field.boxedAt(0); @@ -225,7 +228,7 @@ describe("unhydratedFlexTree", () => { assert.equal(oldMap.parentField.parent.parent, mutableObject); // Allocate a new node - const newMap = flexTreeFromCursor(context, cursorForMapTreeNode(mapMapTree)); + const newMap = unhydratedFlexTreeFromCursor(context, cursorForMapTreeNode(mapMapTree)); assert.notEqual(newMap, oldMap); assert.equal(newMap.parentField.parent.parent, undefined); @@ -243,12 +246,15 @@ describe("unhydratedFlexTree", () => { }); it("optional fields", () => { - const mutableMap = flexTreeFromCursor(context, cursorForMapTreeNode(mapMapTree)); + const mutableMap = unhydratedFlexTreeFromCursor( + context, + cursorForMapTreeNode(mapMapTree), + ); const field = mutableMap.getBoxed(mapKey) as UnhydratedOptionalField; const oldValue = field.boxedAt(0); const newValue = `new ${childValue}`; const newTree: MapTree = { ...mapChildMapTree, value: newValue }; - field.editor.set(flexTreeFromCursor(context, cursorForMapTreeNode(newTree))); + field.editor.set(unhydratedFlexTreeFromCursor(context, cursorForMapTreeNode(newTree))); assert.equal(field.boxedAt(0)?.value, newValue); assert.notEqual(newValue, oldValue); field.editor.set(undefined); @@ -257,7 +263,7 @@ describe("unhydratedFlexTree", () => { describe("arrays", () => { it("insert and remove", () => { - const mutableFieldNode = flexTreeFromCursor( + const mutableFieldNode = unhydratedFlexTreeFromCursor( context, cursorForMapTreeNode(fieldNodeMapTree), ); @@ -268,14 +274,14 @@ describe("unhydratedFlexTree", () => { const treeC: MapTree = { ...mapChildMapTree, value: "c" }; const treeD: MapTree = { ...mapChildMapTree, value: "d" }; field.editor.insert(1, [ - flexTreeFromCursor(context, cursorForMapTreeNode(treeC)), - flexTreeFromCursor(context, cursorForMapTreeNode(treeD)), + unhydratedFlexTreeFromCursor(context, cursorForMapTreeNode(treeC)), + unhydratedFlexTreeFromCursor(context, cursorForMapTreeNode(treeD)), ]); const treeA: MapTree = { ...mapChildMapTree, value: "a" }; const treeB: MapTree = { ...mapChildMapTree, value: "b" }; field.editor.insert(0, [ - flexTreeFromCursor(context, cursorForMapTreeNode(treeA)), - flexTreeFromCursor(context, cursorForMapTreeNode(treeB)), + unhydratedFlexTreeFromCursor(context, cursorForMapTreeNode(treeA)), + unhydratedFlexTreeFromCursor(context, cursorForMapTreeNode(treeB)), ]); assert.deepEqual(values(), ["a", "b", childValue, "c", "d"]); field.editor.remove(2, 1); @@ -284,7 +290,7 @@ describe("unhydratedFlexTree", () => { it("with a large sequence of new content", () => { // This exercises a special code path for inserting large arrays, since large arrays are treated differently to avoid overflow with `splice` + spread. - const mutableFieldNode = flexTreeFromCursor( + const mutableFieldNode = unhydratedFlexTreeFromCursor( context, cursorForMapTreeNode({ ...fieldNodeMapTree, @@ -296,7 +302,7 @@ describe("unhydratedFlexTree", () => { const newContent: UnhydratedFlexTreeNode[] = []; for (let i = 0; i < 10000; i++) { const tree: MapTree = { ...mapChildMapTree, value: String(i) }; - newContent.push(flexTreeFromCursor(context, cursorForMapTreeNode(tree))); + newContent.push(unhydratedFlexTreeFromCursor(context, cursorForMapTreeNode(tree))); } field.editor.insert(0, newContent); assert.equal(field.length, newContent.length); diff --git a/packages/dds/tree/src/test/simple-tree/unhydratedNode.spec.ts b/packages/dds/tree/src/test/simple-tree/unhydratedNode.spec.ts index 61b25ea11d62..cb9d824c3f72 100644 --- a/packages/dds/tree/src/test/simple-tree/unhydratedNode.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/unhydratedNode.spec.ts @@ -29,7 +29,7 @@ import { validateUsageError } from "../utils.js"; import { UnhydratedFlexTreeNode } from "../../simple-tree/core/unhydratedFlexTree.js"; import { singleJsonCursor } from "../json/index.js"; // eslint-disable-next-line import/no-internal-modules -import { flexTreeFromCursor } from "../../simple-tree/api/create.js"; +import { unhydratedFlexTreeFromCursor } from "../../simple-tree/api/create.js"; // eslint-disable-next-line import/no-internal-modules import { getUnhydratedContext } from "../../simple-tree/createContext.js"; @@ -285,7 +285,7 @@ describe("Unhydrated nodes", () => { }); it("flexTreeFromCursor", () => { - const tree = flexTreeFromCursor( + const tree = unhydratedFlexTreeFromCursor( getUnhydratedContext(SchemaFactory.number), singleJsonCursor(1), ); @@ -297,7 +297,7 @@ describe("Unhydrated nodes", () => { const defaultValue = 3; const constantProvider: ConstantFieldProvider = (): UnhydratedFlexTreeNode[] => { return [ - flexTreeFromCursor( + unhydratedFlexTreeFromCursor( getUnhydratedContext(SchemaFactory.number), singleJsonCursor(defaultValue), ), @@ -318,7 +318,7 @@ describe("Unhydrated nodes", () => { const contextualProvider: ContextualFieldProvider = (context: unknown) => { assert.equal(context, "UseGlobalContext"); return [ - flexTreeFromCursor( + unhydratedFlexTreeFromCursor( getUnhydratedContext(SchemaFactory.number), singleJsonCursor(defaultValue), ), From f45634c5da1b1300c01c214d9281bd2f7452dea3 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Mon, 2 Jun 2025 17:15:51 -0700 Subject: [PATCH 22/25] Document globalIdentifierAllocator --- packages/dds/tree/src/simple-tree/api/schemaFactory.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/dds/tree/src/simple-tree/api/schemaFactory.ts b/packages/dds/tree/src/simple-tree/api/schemaFactory.ts index 9cf1950827b1..14fbd1d67e19 100644 --- a/packages/dds/tree/src/simple-tree/api/schemaFactory.ts +++ b/packages/dds/tree/src/simple-tree/api/schemaFactory.ts @@ -1308,4 +1308,10 @@ export function structuralName( return `${collectionName}<${inner}>`; } +/** + * Used to allocate default identifiers for unhydrated nodes when no context is available. + * @remarks + * The identifiers allocated by this will never be compressed to Short Ids. + * Using this is only better than creating fully random V4 UUIDs because it reduces the entropy making it possible for things like text compression to work slightly better. + */ const globalIdentifierAllocator: IIdCompressor = createIdCompressor(); From dabd054893aebcfb14d3e495980c02d084e0cfcb Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Tue, 3 Jun 2025 15:39:40 -0700 Subject: [PATCH 23/25] Document lazyChildren --- .../dds/tree/src/simple-tree/core/unhydratedFlexTree.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index 2914f16c5ab6..10fdb708dfd3 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -331,6 +331,13 @@ export class UnhydratedFlexTreeField public readonly context: FlexTreeContext, public readonly schema: FieldKindIdentifier, public readonly key: FieldKey, + /** + * The children of this field. + * @remarks + * This is either an array of {@link UnhydratedFlexTreeNode}s or a {@link ContextualFieldProvider} that will be used to populate the children lazily (after which it will become an array). + * See {@link fillPendingDefaults}. + * Note that any fields using a {@link ConstantFieldProvider} should be evaluated before constructing the UnhydratedFlexTreeField. + */ private lazyChildren: UnhydratedFlexTreeNode[] | ContextualFieldProvider, ) { // When this field is created (which only happens one time, because it is cached), all the children become parented for the first time. From f0e50f332ef0f04de92a69c88752dd99e1cafac2 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Tue, 3 Jun 2025 15:41:29 -0700 Subject: [PATCH 24/25] Use ConstructorParameters --- .../dds/tree/src/simple-tree/core/unhydratedFlexTree.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index 10fdb708dfd3..eb92eb254b32 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -546,12 +546,7 @@ export class UnhydratedSequenceField /** Creates a field with the given attributes */ export function createField( - ...args: [ - FlexTreeContext, - FieldKindIdentifier, - FieldKey, - UnhydratedFlexTreeNode[] | ContextualFieldProvider, - ] + ...args: ConstructorParameters ): UnhydratedFlexTreeField { switch (args[1]) { case FieldKinds.required.identifier: From 900f39b823445e5d16f5d1c18afba36692559487 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Fri, 6 Jun 2025 09:40:23 -0700 Subject: [PATCH 25/25] Better pending default docs --- .../tree/src/simple-tree/core/unhydratedFlexTree.ts | 4 ++++ .../dds/tree/src/simple-tree/prepareForInsertion.ts | 13 +++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index eb92eb254b32..23e431401142 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -357,6 +357,7 @@ export class UnhydratedFlexTreeField * Populate pending default (if present) using the provided context. * @remarks * This apply to just this field: caller will likely want to recursively walk the tree. + * @see {@link pendingDefault}. */ public fillPendingDefaults(context: FlexTreeHydratedContextMinimal): void { const provider = this.getPendingDefault(); @@ -366,6 +367,9 @@ export class UnhydratedFlexTreeField } } + /** + * Returns true if this field has a pending default due to defined defined using a {@link ContextualFieldProvider}. + */ public get pendingDefault(): boolean { return this.getPendingDefault() !== undefined; } diff --git a/packages/dds/tree/src/simple-tree/prepareForInsertion.ts b/packages/dds/tree/src/simple-tree/prepareForInsertion.ts index 4036bdd597ca..b583cf4529dd 100644 --- a/packages/dds/tree/src/simple-tree/prepareForInsertion.ts +++ b/packages/dds/tree/src/simple-tree/prepareForInsertion.ts @@ -125,8 +125,8 @@ function validateAndPrepare( mapTrees: readonly UnhydratedFlexTreeNode[], ): void { if (hydratedData !== undefined) { - // Prepare content before validating side this populated defaults using the provided context rather than the global context. - // This ensures that when validation requests identifiers (or any other contextual defaults), + // Run `prepareContentForHydration` before walking the tree in `isFieldInSchema`. + // This ensures that when `isFieldInSchema` requests identifiers (or any other contextual defaults), // they were already creating used the more specific context we have access to from `hydratedData`. prepareContentForHydration(mapTrees, hydratedData.checkout.forest, hydratedData); if (schemaAndPolicy.policy.validateSchema === true) { @@ -170,15 +170,16 @@ interface LocatedNodesBatch { const placeholderKey: DetachedField & FieldKey = brand("placeholder" as const); /** - * Records any proxies in the given content tree and does the necessary bookkeeping to ensure they are synchronized with subsequent reads of the tree. - * @remarks If the content tree contains any proxies, this function must be called just prior to inserting the content into the tree. + * Records any {@link TreeNode}s in the given `content` tree and does the necessary bookkeeping to ensure they are synchronized with subsequent reads of the tree. + * Additionally populates any {@link UnhydratedFlexTreeField.pendingDefault}s using the provided `context`. + * + * @remarks If the content tree contains has any associated {@link TreeNode}s, this function must be called just prior to inserting the content into the tree. * Specifically, no other content may be inserted into the tree between the invocation of this function and the insertion of `content`. * The insertion of `content` must occur or else this function will cause memory leaks. * - * Exported fot testing purposes: otherwise should not be used outside this module. + * Exported for testing purposes: otherwise should not be used outside this module. * @param content - the content subsequence to be inserted, of which might deeply contain {@link TreeNode}s which need to be hydrated. * @param forest - the forest the content is being inserted into. - * See {@link extractFactoryContent} for more details. */ export function prepareContentForHydration( content: readonly UnhydratedFlexTreeNode[],