Skip to content

Commit

Permalink
Rename MapTreeNode to UnhydratedFlexTreeNode (microsoft#22623)
Browse files Browse the repository at this point in the history
## Description

To avoid confusion with MapNodes, and MapTrees, MapTreeNodes, which are
the unhydrated implementation of the FlexTreeNode interface, have been
renamed to UnhydratedFlexTreeNode.

Additionally, the implementation has been moved simple-tree/core where
it can now hold onto an UnhydratedContext (from simple-tree), filling
the last gap (when lazilly allocating TreeNodes for children of
unhydrated nodes) where there was no easy way to get the simple-tree
schema when required without depending on the flex-tree schema.
  • Loading branch information
CraigMacomber authored Sep 25, 2024
1 parent 8077539 commit a255817
Show file tree
Hide file tree
Showing 18 changed files with 204 additions and 299 deletions.
14 changes: 0 additions & 14 deletions packages/dds/tree/src/feature-libraries/flex-map-tree/index.ts

This file was deleted.

12 changes: 2 additions & 10 deletions packages/dds/tree/src/feature-libraries/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ export {
isFreedSymbol,
LazyEntity,
treeStatusFromAnchorCache,
indexForAt,
FlexTreeEntityKind,
} from "./flex-tree/index.js";

export { treeSchemaFromStoredSchema } from "./storedToViewSchema.js";
Expand All @@ -237,13 +239,3 @@ export {
} from "./schema-edits/index.js";

export { makeMitigatedChangeFamily } from "./mitigatedChangeFamily.js";

export {
type MapTreeNode,
type MapTreeSequenceField,
isMapTreeNode,
isMapTreeSequenceField,
getOrCreateMapTreeNode,
tryGetMapTreeNode,
UnhydratedContext,
} from "./flex-map-tree/index.js";
16 changes: 9 additions & 7 deletions packages/dds/tree/src/simple-tree/api/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import type { IFluidHandle } from "@fluidframework/core-interfaces";
import { assert } from "@fluidframework/core-utils/internal";

import type { ITreeCursorSynchronous, SchemaAndPolicy } from "../../core/index.js";
import { fail } from "../../util/index.js";
import type {
TreeLeafValue,
ImplicitFieldSchema,
Expand All @@ -16,17 +15,21 @@ import type {
FieldSchema,
FieldKind,
} from "../schemaTypes.js";
import { getOrCreateNodeFromInnerNode, type Unhydrated } from "../core/index.js";
import {
getOrCreateNodeFromInnerNode,
UnhydratedFlexTreeNode,
type Unhydrated,
UnhydratedContext,
} from "../core/index.js";
import {
cursorForMapTreeNode,
defaultSchemaPolicy,
FieldKinds,
intoStoredSchema,
mapTreeFromCursor,
UnhydratedContext,
type NodeKeyManager,
} from "../../feature-libraries/index.js";
import { getOrCreateMapTreeNode, isFieldInSchema } from "../../feature-libraries/index.js";
import { isFieldInSchema } from "../../feature-libraries/index.js";
import { toFlexSchema } from "../toFlexSchema.js";
import { inSchemaOrThrow, mapTreeFromNodeData, type InsertableContent } from "../toMapTree.js";
import {
Expand Down Expand Up @@ -170,10 +173,9 @@ export function createFromCursor<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]!;
const rootSchema = flexSchema.nodeSchema.get(mapTree.type) ?? fail("missing schema");
const mapTreeNode = getOrCreateMapTreeNode(
const mapTreeNode = UnhydratedFlexTreeNode.getOrCreate(
// TODO: Provide a way to get simple-tree context here, then make UnhydratedFlexTreeNode's hold simple-tree contexts. Use this for InnerNode -> TreeSchemaSchema
new UnhydratedContext(flexSchema),
rootSchema,
mapTree,
);

Expand Down
4 changes: 2 additions & 2 deletions packages/dds/tree/src/simple-tree/api/treeNodeApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
isLazy,
isTreeValue,
FlexObjectNodeSchema,
isMapTreeNode,
FieldKinds,
} from "../../feature-libraries/index.js";
import { fail, extractFromOpaque, isReadonlyArray } from "../../util/index.js";
Expand Down Expand Up @@ -43,6 +42,7 @@ import {
type TreeChangeEvents,
tryGetTreeNodeSchema,
getOrCreateNodeFromInnerNode,
UnhydratedFlexTreeNode,
} from "../core/index.js";
import { isObjectNodeSchema } from "../objectNodeTypes.js";

Expand Down Expand Up @@ -240,7 +240,7 @@ export const treeNodeApi: TreeNodeApi = {
return undefined;
case 1: {
const identifier = flexNode.tryGetField(identifierFieldKeys[0] ?? oob())?.boxedAt(0);
if (isMapTreeNode(flexNode)) {
if (flexNode instanceof UnhydratedFlexTreeNode) {
if (identifier === undefined) {
throw new UsageError(
"Tree.shortId cannot access default identifiers on unhydrated nodes",
Expand Down
30 changes: 14 additions & 16 deletions packages/dds/tree/src/simple-tree/arrayNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,13 @@ import { UsageError } from "@fluidframework/telemetry-utils/internal";

import { EmptyKey, type ExclusiveMapTree } from "../core/index.js";
import {
type FlexTreeNodeSchema,
type FlexTreeNode,
type FlexTreeSequenceField,
type MapTreeNode,
getOrCreateMapTreeNode,
getSchemaAndPolicy,
isFlexTreeNode,
isMapTreeSequenceField,
} from "../feature-libraries/index.js";
import { prepareContentForHydration } from "./proxies.js";
import { getOrCreateInnerNode } from "./proxyBinding.js";
// This import seems to trigger a false positive `Type import "TreeNodeFromImplicitAllowedTypes" is used by decorator metadata` lint error.
// Other ways to import (ex: import the module with the items as type imports) give different more real errors, and auto fix to this format,
// so there does not seem to be a clean workaround to make the linter happy.
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
import {
normalizeAllowedTypes,
type ImplicitAllowedTypes,
Expand All @@ -44,8 +36,11 @@ import {
} from "./core/index.js";
import { type InsertableContent, mapTreeFromNodeData } from "./toMapTree.js";
import { fail } from "../util/index.js";
import { getFlexSchema } from "./toFlexSchema.js";
import { getKernel } from "./core/index.js";
import {
getKernel,
UnhydratedFlexTreeNode,
UnhydratedTreeSequenceField,
} from "./core/index.js";
import { TreeNodeValid, type MostDerivedData } from "./treeNodeValid.js";
import { createUnhydratedContext } from "./createContext.js";

Expand Down Expand Up @@ -841,7 +836,13 @@ abstract class CustomArrayNodeBase<const T extends ImplicitAllowedTypes>

const movedCount = sourceEnd - sourceStart;
if (!destinationField.context.isHydrated()) {
if (!isMapTreeSequenceField(sourceField)) {
if (!(sourceField instanceof UnhydratedTreeSequenceField)) {
throw new UsageError(
"Cannot move elements from an unhydrated array to a hydrated array.",
);
}

if (sourceField.context.isHydrated()) {
throw new UsageError(
"Cannot move elements from an unhydrated array to a hydrated array.",
);
Expand Down Expand Up @@ -924,7 +925,6 @@ export function arraySchema<

const lazyChildTypes = new Lazy(() => normalizeAllowedTypes(info));

let flexSchema: FlexTreeNodeSchema;
let unhydratedContext: Context;

// This class returns a proxy from its constructor to handle numeric indexing.
Expand Down Expand Up @@ -954,10 +954,9 @@ export function arraySchema<
this: typeof TreeNodeValid<T2>,
instance: TreeNodeValid<T2>,
input: T2,
): MapTreeNode {
return getOrCreateMapTreeNode(
): UnhydratedFlexTreeNode {
return UnhydratedFlexTreeNode.getOrCreate(
unhydratedContext.flexContext,
flexSchema,
mapTreeFromNodeData(input as object, this as unknown as ImplicitAllowedTypes),
);
}
Expand All @@ -966,7 +965,6 @@ export function arraySchema<

protected static override oneTimeSetup<T2>(this: typeof TreeNodeValid<T2>): Context {
const schema = this as unknown as TreeNodeSchema;
flexSchema = getFlexSchema(schema);
unhydratedContext = createUnhydratedContext(schema);

// First run, do extra validation.
Expand Down
10 changes: 6 additions & 4 deletions packages/dds/tree/src/simple-tree/core/getOrCreateNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
*/

import type { TreeValue } from "../../core/index.js";
import { isMapTreeNode, type FlexTreeNode } from "../../feature-libraries/index.js";
import type { FlexTreeNode } from "../../feature-libraries/index.js";
import { fail } from "../../util/index.js";
import { type InnerNode, mapTreeNodeToProxy, proxySlot } from "./treeNodeKernel.js";
import { getSimpleNodeSchemaFromInnerNode } from "./schemaCaching.js";
import type { TreeNode, InternalTreeNode } from "./types.js";
import { UnhydratedFlexTreeNode } from "./unhydratedFlexTree.js";

/**
* Returns the TreeNode or TreeValue for the provided {@link InnerNode}.
Expand All @@ -17,9 +18,10 @@ import type { TreeNode, InternalTreeNode } from "./types.js";
* This supports both hydrated and unhydrated nodes.
*/
export function getOrCreateNodeFromInnerNode(flexNode: InnerNode): TreeNode | TreeValue {
const cached = isMapTreeNode(flexNode)
? mapTreeNodeToProxy.get(flexNode)
: flexNode.anchorNode.slots.get(proxySlot);
const cached =
flexNode instanceof UnhydratedFlexTreeNode
? mapTreeNodeToProxy.get(flexNode)
: flexNode.anchorNode.slots.get(proxySlot);

if (cached !== undefined) {
return cached;
Expand Down
6 changes: 6 additions & 0 deletions packages/dds/tree/src/simple-tree/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,9 @@ export {
export { walkAllowedTypes, type SchemaVisitor } from "./walkSchema.js";
export { Context, HydratedContext, SimpleContextSlot } from "./context.js";
export { getOrCreateNodeFromInnerNode } from "./getOrCreateNode.js";
export {
UnhydratedFlexTreeNode,
UnhydratedTreeSequenceField,
tryUnhydratedFlexTreeNode,
UnhydratedContext,
} from "./unhydratedFlexTree.js";
19 changes: 10 additions & 9 deletions packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,18 @@ import {
flexTreeSlot,
isFlexTreeNode,
isFreedSymbol,
isMapTreeNode,
LazyEntity,
TreeStatus,
treeStatusFromAnchorCache,
type FlexTreeNode,
type MapTreeNode,
} from "../../feature-libraries/index.js";
import type { TreeNodeSchema } from "./treeNodeSchema.js";
import { fail } from "../../util/index.js";
// TODO: decide how to deal with dependencies on flex-tree implementation.
// eslint-disable-next-line import/no-internal-modules
import { makeTree } from "../../feature-libraries/flex-tree/lazyNode.js";
import { SimpleContextSlot, type Context } from "./context.js";
import { UnhydratedFlexTreeNode } from "./unhydratedFlexTree.js";

const treeNodeToKernel = new WeakMap<TreeNode, TreeNodeKernel>();

Expand Down Expand Up @@ -149,7 +148,7 @@ export class TreeNodeKernel implements Listenable<KernelEvents> {
assert(!treeNodeToKernel.has(node), 0xa1a /* only one kernel per node can be made */);
treeNodeToKernel.set(node, this);

if (isMapTreeNode(innerNode)) {
if (innerNode instanceof UnhydratedFlexTreeNode) {
// Unhydrated case
mapTreeNodeToProxy.set(innerNode, node);
} else {
Expand Down Expand Up @@ -178,14 +177,14 @@ export class TreeNodeKernel implements Listenable<KernelEvents> {
* Bi-directionally associates the given hydrated TreeNode to the given anchor node.
* @remarks
* Happens at most once for any given node.
* Cleans up mappings to {@link MapTreeNode} - it is assumed that they are no longer needed once the proxy has an anchor node.
* Cleans up mappings to {@link UnhydratedFlexTreeNode} - it is assumed that they are no longer needed once the proxy has an anchor node.
*/
public hydrate(anchorNode: AnchorNode): void {
assert(!this.disposed, 0xa2a /* cannot hydrate a disposed node */);
assert(this.#hydrated === undefined, 0xa2b /* hydration should only happen once */);

// If the this node is raw and thus has a MapTreeNode, forget it:
if (isMapTreeNode(this.innerNode)) {
if (this.innerNode instanceof UnhydratedFlexTreeNode) {
mapTreeNodeToProxy.delete(this.innerNode);
}

Expand Down Expand Up @@ -264,7 +263,7 @@ export class TreeNodeKernel implements Listenable<KernelEvents> {
* Note that for "marinated" nodes, this FlexTreeNode exists and returns it: it does not return the MapTreeNode which is the current InnerNode.
*/
public getOrCreateInnerNode(allowFreed = false): InnerNode {
if (!isMapTreeNode(this.innerNode)) {
if (!(this.innerNode instanceof UnhydratedFlexTreeNode)) {
// Cooked case
return this.innerNode;
}
Expand Down Expand Up @@ -370,12 +369,12 @@ type KernelEvents = Pick<AnchorEvents, (typeof kernelEvents)[number]>;
* TODO: The inconsistent handling of "marinated" cases should be cleaned up.
* Maybe getOrCreateInnerNode should cook marinated nodes so they have a proper InnerNode?
*/
export type InnerNode = FlexTreeNode | MapTreeNode;
export type InnerNode = FlexTreeNode | UnhydratedFlexTreeNode;

/**
* {@inheritdoc proxyToMapTreeNode}
*/
export const mapTreeNodeToProxy = new WeakMap<MapTreeNode, TreeNode>();
export const mapTreeNodeToProxy = new WeakMap<UnhydratedFlexTreeNode, TreeNode>();

/**
* An anchor slot which associates an anchor with its corresponding TreeNode, if there is one.
Expand All @@ -388,7 +387,9 @@ export const proxySlot = anchorSlot<TreeNode>();
/**
* Retrieves the node associated with the given MapTreeNode node if any.
*/
export function tryGetTreeNodeFromMapNode(flexNode: MapTreeNode): TreeNode | undefined {
export function tryGetTreeNodeFromMapNode(
flexNode: UnhydratedFlexTreeNode,
): TreeNode | undefined {
return mapTreeNodeToProxy.get(flexNode);
}

Expand Down
Loading

0 comments on commit a255817

Please sign in to comment.