refactor: Store data in unhydrated nodes directly instead of wrapping MapTree nodes

.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.

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, mapIterable } 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)) {
@@ -69,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);

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";
@@ -287,7 +287,7 @@ 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.

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 {

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

@@ -40,11 +40,11 @@ export interface MapTreeNodeViewGeneric<TNode> 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<FieldKey, MapTreeFieldViewGeneric<TNode>>,
-		typeof Symbol.iterator | "get" | "size" | "keys"
+		Map<FieldKey, MapTreeFieldViewGeneric<TNode>>,
+		typeof Symbol.iterator | "get"
 	>;
 }
 
@@ -127,7 +127,7 @@ export function cursorForMapTreeField<T extends MapTreeNodeViewGeneric<T>>(
 const adapter: CursorAdapter<MinimalMapTreeNodeView> = {
 	value: (node) => node.value,
 	type: (node) => node.type,
-	keysFromNode: (node) => [...node.fields.keys()], // TODO: don't convert this to array here.
+	keysFromNode: (node) => Array.from(node.fields, ([key, field]) => key),
 	getFieldFromNode: (node, key): Field<MinimalMapTreeNodeView> => {
 		const field = node.fields.get(key) as
 			| MinimalMapTreeNodeView[]

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,7 +59,6 @@ import {
 import { independentInitializedView, type ViewContent } from "./independentView.js";
 import { SchematizingSimpleTreeView, ViewSlot } from "./schematizingTreeView.js";
 import { currentVersion, noopValidator } from "../codec/index.js";
-import { createFromMapTree } from "../simple-tree/index.js";
 
 const identifier: TreeIdentifierUtils = (node: TreeNode): string | undefined => {
 	const nodeIdentifier = getIdentifierFromNode(node, "uncompressed");
@@ -371,7 +371,7 @@ export const TreeAlpha: TreeAlpha = {
 			: TreeNode | TreeLeafValue | undefined
 	> {
 		const mapTree = mapTreeFromNodeData(data as InsertableField<UnsafeUnknownSchema>, schema);
-		const result = mapTree === undefined ? undefined : createFromMapTree(schema, mapTree);
+		const result = mapTree === undefined ? undefined : getOrCreateNodeFromInnerNode(mapTree);
 		return result as Unhydrated<
 			TSchema extends ImplicitFieldSchema
 				? TreeFieldFromImplicitField<TSchema>

packages/dds/tree/src/simple-tree/api/create.ts

@@ -5,35 +5,46 @@
 
 import { assert } from "@fluidframework/core-utils/internal";
 
-import type {
-	ExclusiveMapTree,
-	ITreeCursorSynchronous,
-	SchemaAndPolicy,
+import {
+	CursorLocationType,
+	mapCursorField,
+	mapCursorFields,
+	type ITreeCursorSynchronous,
+	type SchemaAndPolicy,
 } from "../../core/index.js";
 import type { ImplicitFieldSchema, TreeFieldFromImplicitField } from "../schemaTypes.js";
 import {
+	type Context,
 	getOrCreateNodeFromInnerNode,
-	UnhydratedFlexTreeNode,
+	type NodeKind,
 	type Unhydrated,
+	UnhydratedFlexTreeNode,
+	createField,
 } 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";
+import { getStoredSchema } from "../toStoredSchema.js";
+import { unknownTypeError } from "./customTree.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<const TSchema extends ImplicitFieldSchema>(
 	schema: TSchema,
 	cursor: ITreeCursorSynchronous | undefined,
 ): Unhydrated<TreeFieldFromImplicitField<TSchema>> {
-	const mapTrees = cursor === undefined ? [] : [mapTreeFromCursor(cursor)];
 	const context = getUnhydratedContext(schema);
+	const mapTrees = cursor === undefined ? [] : [unhydratedFlexTreeFromCursor(context, cursor)];
+
 	const flexSchema = context.flexContext.schema;
 
 	const schemaValidationPolicy: SchemaAndPolicy = {
@@ -58,21 +69,40 @@ export function createFromCursor<const TSchema extends ImplicitFieldSchema>(
 	// 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);
+
+	return getOrCreateNodeFromInnerNode(mapTree) as Unhydrated<
+		TreeFieldFromImplicitField<TSchema>
+	>;
 }
 
 /**
- * Creates an unhydrated simple-tree field from an ExclusiveMapTree.
+ * Construct an {@link UnhydratedFlexTreeNode} from a cursor in Nodes mode.
+ * @remarks
+ * This does not validate the node is in schema.
  */
-export function createFromMapTree<const TSchema extends ImplicitFieldSchema>(
-	schema: TSchema,
-	mapTree: ExclusiveMapTree,
-): Unhydrated<TreeFieldFromImplicitField<TSchema>> {
-	const mapTreeNode = UnhydratedFlexTreeNode.getOrCreate(
-		getUnhydratedContext(schema),
-		mapTree,
+export function unhydratedFlexTreeFromCursor(
+	context: Context,
+	cursor: ITreeCursorSynchronous,
+): UnhydratedFlexTreeNode {
+	assert(cursor.mode === CursorLocationType.Nodes, "Expected nodes cursor");
+	const schema = context.schema.get(cursor.type) ?? unknownTypeError(cursor.type);
+	const storedSchema = getStoredSchema(
+		schema as SimpleNodeSchemaBase<NodeKind> as SimpleNodeSchema,
+	);
+	const fields = new Map(
+		mapCursorFields(cursor, () => [
+			cursor.getFieldKey(),
+			createField(
+				context.flexContext,
+				storedSchema.getFieldSchema(cursor.getFieldKey()).kind,
+				cursor.getFieldKey(),
+				mapCursorField(cursor, () => unhydratedFlexTreeFromCursor(context, cursor)),
+			),
+		]),
+	);
+	return new UnhydratedFlexTreeNode(
+		{ type: cursor.type, value: cursor.value },
+		fields,
+		context,
 	);
-
-	const result = getOrCreateNodeFromInnerNode(mapTreeNode);
-	return result as Unhydrated<TreeFieldFromImplicitField<TSchema>>;
 }

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,

packages/dds/tree/src/simple-tree/api/schemaFactory.ts

@@ -9,7 +9,6 @@ import { isFluidHandle } from "@fluidframework/runtime-utils/internal";
 import { UsageError } from "@fluidframework/telemetry-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";
@@ -26,6 +25,7 @@ import type {
 	TreeNodeSchemaClass,
 	TreeNodeSchemaNonClass,
 	TreeNodeSchemaBoth,
+	UnhydratedFlexTreeNode,
 } from "../core/index.js";
 import { isLazy } from "../flexList.js";
 import {
@@ -67,6 +67,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}.
@@ -296,9 +300,7 @@ export interface SchemaStatics {
 	) => System_Unsafe.FieldSchemaUnsafe<FieldKind.Required, T, TCustomMetadata>;
 }
 
-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<T> is equal to T when T is known to extend ImplicitAllowedTypes
@@ -1112,8 +1114,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.
@@ -1122,10 +1122,19 @@ export class SchemaFactory<
 	 */
 	public get identifier(): FieldSchema<FieldKind.Identifier, typeof this.string> {
 		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, {
@@ -1298,3 +1307,11 @@ export function structuralName<const T extends string>(
 	}
 	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();