Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tree2: Remove FieldNode unboxing #18260

Merged
merged 1 commit into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions experimental/dds/tree2/api-report/tree2.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -2096,7 +2096,7 @@ type UnboxField<TSchema extends TreeFieldSchema, Emptiness extends "maybeEmpty"
type UnboxFieldInner<Kind extends FieldKind, TTypes extends AllowedTypes, Emptiness extends "maybeEmpty" | "notEmpty"> = Kind extends typeof FieldKinds.sequence ? Sequence<TTypes> : Kind extends typeof FieldKinds.required ? UnboxNodeUnion<TTypes> : Kind extends typeof FieldKinds.optional ? UnboxNodeUnion<TTypes> | (Emptiness extends "notEmpty" ? never : undefined) : Kind extends typeof FieldKinds.nodeKey ? NodeKeyField : unknown;

// @alpha
type UnboxNode<TSchema extends TreeNodeSchema> = TSchema extends LeafSchema ? TreeValue<TSchema["leafValue"]> : TSchema extends MapSchema ? MapNode<TSchema> : TSchema extends FieldNodeSchema ? UnboxField<TSchema["objectNodeFieldsObject"][""]> : TSchema extends ObjectNodeSchema ? ObjectNodeTyped<TSchema> : UnknownUnboxed;
type UnboxNode<TSchema extends TreeNodeSchema> = TSchema extends LeafSchema ? TreeValue<TSchema["leafValue"]> : TSchema extends MapSchema ? MapNode<TSchema> : TSchema extends FieldNodeSchema ? FieldNode<TSchema> : TSchema extends ObjectNodeSchema ? ObjectNodeTyped<TSchema> : UnknownUnboxed;

// @alpha
type UnboxNodeUnion<TTypes extends AllowedTypes> = TTypes extends readonly [
Expand All @@ -2118,7 +2118,7 @@ type UnbrandList<T extends unknown[], B> = T extends [infer Head, ...infer Tail]
export type Unenforced<_DesiredExtendsConstraint> = unknown;

// @alpha
type UnknownUnboxed = TreeValue | TreeNode | TreeField;
type UnknownUnboxed = TreeValue | TreeNode;

// @alpha
type UntypedApi<Mode extends ApiMode> = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,9 +348,7 @@ export interface MapNode<in out TSchema extends MapSchema> extends TreeNode {
* A {@link TreeNode} that wraps a single {@link TreeField} (which is placed under the {@link EmptyKey}).
*
* @remarks
* FieldNodes unbox to their content, so in schema aware APIs which do unboxing, the FieldNode itself will be skipped over.
* This layer of field nodes is then omitted when using schema-aware APIs which do unboxing.
* Other than this unboxing, a FieldNode is identical to a struct node with a single field using the {@link EmptyKey}.
* A FieldNode is mostly identical to a struct node with a single field using the {@link EmptyKey}, but provides access to it via a field named "content".
*
* There are several use-cases where it makes sense to use a field node.
* Here are a few:
Expand All @@ -377,10 +375,8 @@ export interface MapNode<in out TSchema extends MapSchema> extends TreeNode {
* `FieldNode<Sequence<Foo>> | FieldNode<Sequence<Bar>>` or `OptionalField<FieldNode<Sequence<Foo>>>`.
*
* @privateRemarks
* TODO: The rule walking over the tree via enumerable own properties is lossless (see [ReadMe](./README.md) for details)
* fails to be true for recursive field nodes with field kind optional, since the length of the chain of field nodes is lost.
* THis could be fixed by tweaking the unboxing rules, or simply ban view schema that would have this problem (check for recursive optional field nodes).
* Replacing the field node pattern with one where the FieldNode node exposes APIs from its field instead of unboxing could have the same issue, and same solutions.
* FieldNodes do not unbox to their content, so in schema aware APIs which do unboxing, the FieldNode will NOT be skipped over.
* This is a change from the old behavior to simplify unboxing and prevent cases where arbitrary deep chains of field nodes could unbox omitting information about the tree depth.
* @alpha
*/
export interface FieldNode<in out TSchema extends FieldNodeSchema> extends TreeNode {
Expand Down Expand Up @@ -991,7 +987,7 @@ export type UnboxNode<TSchema extends TreeNodeSchema> = TSchema extends LeafSche
: TSchema extends MapSchema
? MapNode<TSchema>
: TSchema extends FieldNodeSchema
? UnboxField<TSchema["objectNodeFieldsObject"][""]>
? FieldNode<TSchema>
: TSchema extends ObjectNodeSchema
? ObjectNodeTyped<TSchema>
: UnknownUnboxed;
Expand All @@ -1000,6 +996,6 @@ export type UnboxNode<TSchema extends TreeNodeSchema> = TSchema extends LeafSche
* Unboxed tree type for unknown schema cases.
* @alpha
*/
export type UnknownUnboxed = TreeValue | TreeNode | TreeField;
export type UnknownUnboxed = TreeValue | TreeNode;

// #endregion
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,12 @@
* Licensed under the MIT License.
*/

import { ITreeSubscriptionCursor, inCursorNode, EmptyKey } from "../../core";
import { ITreeSubscriptionCursor, inCursorNode } from "../../core";
import { FieldKind } from "../modular-schema";
import { FieldKinds } from "../default-field-kinds";
import { fail } from "../../util";
import {
AllowedTypes,
TreeFieldSchema,
TreeNodeSchema,
schemaIsFieldNode,
schemaIsLeaf,
} from "../typed-schema";
import { AllowedTypes, TreeFieldSchema, TreeNodeSchema, schemaIsLeaf } from "../typed-schema";
import { Context } from "./context";
import { UnboxField, UnboxNode, UnboxNodeUnion } from "./editableTreeTypes";
import { TreeNode, UnboxField, UnboxNode, UnboxNodeUnion } from "./editableTreeTypes";
import { makeTree } from "./lazyTree";
import { makeField } from "./lazyField";

Expand All @@ -30,18 +23,8 @@ export function unboxedTree<TSchema extends TreeNodeSchema>(
if (schemaIsLeaf(schema)) {
return cursor.value as UnboxNode<TSchema>;
}
if (schemaIsFieldNode(schema)) {
cursor.enterField(EmptyKey);
const primaryField = makeField(
context,
schema.objectNodeFields.get(EmptyKey) ?? fail("invalid schema"),
cursor,
);
cursor.exitField();
return primaryField as UnboxNode<TSchema>;
}

return makeTree(context, cursor) as UnboxNode<TSchema>;
return makeTree(context, cursor) as TreeNode as UnboxNode<TSchema>;
}

/**
Expand Down
18 changes: 9 additions & 9 deletions experimental/dds/tree2/src/test/domains/schemaBuilder.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
SharedTreeObject,
} from "../../feature-libraries";
// eslint-disable-next-line import/no-internal-modules
import { TypedNode, UnboxNode } from "../../feature-libraries/editable-tree-2/editableTreeTypes";
import { TypedNode } from "../../feature-libraries/editable-tree-2/editableTreeTypes";
import { areSafelyAssignable, isAny, requireFalse, requireTrue } from "../../util";
// eslint-disable-next-line import/no-internal-modules
import { structuralName } from "../../domains/schemaBuilder";
Expand All @@ -39,7 +39,7 @@ describe("domains - SchemaBuilder", () => {
.get("")
.equals(TreeFieldSchema.create(FieldKinds.sequence, [Any])),
);
type ListAny = UnboxNode<typeof listAny>;
type ListAny = TypedNode<typeof listAny>["content"];
type _check = requireTrue<areSafelyAssignable<ListAny, Sequence<readonly [Any]>>>;

assert.equal(builder.list(Any), listAny);
Expand All @@ -56,9 +56,9 @@ describe("domains - SchemaBuilder", () => {
.get("")
.equals(TreeFieldSchema.create(FieldKinds.sequence, [builder.number])),
);
type ListAny = UnboxNode<typeof listImplicit>;
type ListImplicit = TypedNode<typeof listImplicit>["content"];
type _check = requireTrue<
areSafelyAssignable<ListAny, Sequence<readonly [typeof builder.number]>>
areSafelyAssignable<ListImplicit, Sequence<readonly [typeof builder.number]>>
>;

assert.equal(builder.list(builder.number), listImplicit);
Expand Down Expand Up @@ -94,18 +94,18 @@ describe("domains - SchemaBuilder", () => {
]),
),
);
type ListAny = UnboxNode<typeof listUnion>;
type ListUnion = TypedNode<typeof listUnion>["content"];
type _check = requireTrue<
areSafelyAssignable<
ListAny,
ListUnion,
Sequence<readonly [typeof builder.number, typeof builder.boolean]>
>
>;
// TODO: this should compile: ideally EditableTree's use of AllowedTypes would be compile time order independent like it is runtime order independent, but its currently not.
type _check2 = requireTrue<
// @ts-expect-error Currently not order independent: ideally this would compile
areSafelyAssignable<
ListAny,
ListUnion,
Sequence<readonly [typeof builder.boolean, typeof builder.number]>
>
>;
Expand All @@ -127,9 +127,9 @@ describe("domains - SchemaBuilder", () => {
.get("")
.equals(TreeFieldSchema.create(FieldKinds.sequence, [builder.number])),
);
type ListAny = UnboxNode<typeof list>;
type List = TypedNode<typeof list>["content"];
type _check = requireTrue<
areSafelyAssignable<ListAny, Sequence<readonly [typeof builder.number]>>
areSafelyAssignable<List, Sequence<readonly [typeof builder.number]>>
>;

// Not cached for structural use
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -381,8 +381,9 @@ describe("editableTreeTypes", () => {
// Unboxed FieldNode
{
type UnboxedFieldNode = UnboxNodeUnion<[typeof basicFieldNode]>;
type _1 = requireTrue<areSafelyAssignable<TreeNode | undefined, UnboxedFieldNode>>;
// @ts-expect-error union can unbox to undefined
type _1 = requireTrue<
areSafelyAssignable<TypedNode<typeof basicFieldNode>, UnboxedFieldNode>
>;
type _2 = requireAssignableTo<UnboxedFieldNode, TreeNode>;
}
// Recursive
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,6 @@ function checkPropertyInvariants(root: TreeEntity): void {

const unboxable = new Set([
LazyLeaf.prototype,
LazyFieldNode.prototype,
LazyValueField.prototype,
LazyOptionalField.prototype,
]);
Expand Down