From ffb1c44fd7071df939c4aa4d68eed9f82309f60d Mon Sep 17 00:00:00 2001 From: Jenn Date: Tue, 17 Dec 2024 22:34:24 +0000 Subject: [PATCH 01/16] move global and rename effects to the delta root object --- packages/dds/tree/src/core/tree/delta.ts | 47 +++++++++++------------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/packages/dds/tree/src/core/tree/delta.ts b/packages/dds/tree/src/core/tree/delta.ts index fc1ad6c8beb6..cda161f78065 100644 --- a/packages/dds/tree/src/core/tree/delta.ts +++ b/packages/dds/tree/src/core/tree/delta.ts @@ -98,6 +98,22 @@ export interface Root { * The ordering has no significance. */ readonly refreshers?: readonly DetachedNodeBuild[]; + /** + * Changes to apply to detached nodes. + * The ordering has no significance. + * + * Nested changes for a root that is undergoing a rename should be listed under the starting name. + * For example, if one wishes to change a tree which is being renamed from ID A to ID B, + * then the changes should be listed under ID A. + */ + readonly global?: readonly DetachedNodeChanges[]; + /** + * Detached whose associated ID needs to be updated. + * The ordering has no significance. + * Note that the renames may need to be performed in a specific order to avoid collisions. + * This ordering problem is left to the consumer of this format. + */ + readonly rename?: readonly DetachedNodeRename[]; } /** @@ -199,30 +215,9 @@ export interface DetachedNodeRename { } /** - * Represents the changes to perform on a given field. + * Represents a list of changes to the nodes in the field. + * The index of each mark within the range of nodes, before + * applying any of the changes, is not represented explicitly. + * It corresponds to the sum of `mark.count` values for all previous marks for which `isAttachMark(mark)` is false. */ -export interface FieldChanges { - /** - * Represents a list of changes to the nodes in the field. - * The index of each mark within the range of nodes, before - * applying any of the changes, is not represented explicitly. - * It corresponds to the sum of `mark.count` values for all previous marks for which `isAttachMark(mark)` is false. - */ - readonly local?: readonly Mark[]; - /** - * Changes to apply to detached nodes. - * The ordering has no significance. - * - * Nested changes for a root that is undergoing a rename should be listed under the starting name. - * For example, if one wishes to change a tree which is being renamed from ID A to ID B, - * then the changes should be listed under ID A. - */ - readonly global?: readonly DetachedNodeChanges[]; - /** - * Detached whose associated ID needs to be updated. - * The ordering has no significance. - * Note that the renames may need to be performed in a specific order to avoid collisions. - * This ordering problem is left to the consumer of this format. - */ - readonly rename?: readonly DetachedNodeRename[]; -} +export type FieldChanges = readonly Mark[]; \ No newline at end of file From 223e3d7c975dc19e43bdbb42a5ef3c9a86dbc09a Mon Sep 17 00:00:00 2001 From: Jenn Date: Tue, 7 Jan 2025 17:56:34 +0000 Subject: [PATCH 02/16] move global and rename into root delta --- packages/dds/tree/src/core/tree/delta.ts | 2 +- packages/dds/tree/src/core/tree/deltaUtil.ts | 15 +- packages/dds/tree/src/core/tree/visitDelta.ts | 166 +++++++++--------- .../forest-summary/forestSummarizer.ts | 7 +- 4 files changed, 87 insertions(+), 103 deletions(-) diff --git a/packages/dds/tree/src/core/tree/delta.ts b/packages/dds/tree/src/core/tree/delta.ts index cda161f78065..072353489746 100644 --- a/packages/dds/tree/src/core/tree/delta.ts +++ b/packages/dds/tree/src/core/tree/delta.ts @@ -220,4 +220,4 @@ export interface DetachedNodeRename { * applying any of the changes, is not represented explicitly. * It corresponds to the sum of `mark.count` values for all previous marks for which `isAttachMark(mark)` is false. */ -export type FieldChanges = readonly Mark[]; \ No newline at end of file +export type FieldChanges = readonly Mark[]; diff --git a/packages/dds/tree/src/core/tree/deltaUtil.ts b/packages/dds/tree/src/core/tree/deltaUtil.ts index 47ca9755fe97..1a5c99aee98b 100644 --- a/packages/dds/tree/src/core/tree/deltaUtil.ts +++ b/packages/dds/tree/src/core/tree/deltaUtil.ts @@ -12,7 +12,7 @@ import { rootFieldKey } from "./types.js"; export const emptyDelta: Root = {}; -export const emptyFieldChanges: FieldChanges = {}; +export const emptyFieldChanges: FieldChanges = []; export function isAttachMark(mark: Mark): boolean { return mark.attach !== undefined && mark.detach === undefined; @@ -27,11 +27,7 @@ export function isReplaceMark(mark: Mark): boolean { } export function isEmptyFieldChanges(fieldChanges: FieldChanges): boolean { - return ( - fieldChanges.local === undefined && - fieldChanges.global === undefined && - fieldChanges.rename === undefined - ); + return fieldChanges.length === 0; } export function deltaForRootInitialization(content: readonly ITreeCursorSynchronous[]): Root { @@ -42,12 +38,7 @@ export function deltaForRootInitialization(content: readonly ITreeCursorSynchron const delta: Root = { build: [{ id: buildId, trees: content }], fields: new Map([ - [ - rootFieldKey, - { - local: [{ count: content.length, attach: buildId }], - }, - ], + [rootFieldKey, [{ count: content.length, attach: buildId }]], ]), }; return delta; diff --git a/packages/dds/tree/src/core/tree/visitDelta.ts b/packages/dds/tree/src/core/tree/visitDelta.ts index af8bd19945da..d259431d55ba 100644 --- a/packages/dds/tree/src/core/tree/visitDelta.ts +++ b/packages/dds/tree/src/core/tree/visitDelta.ts @@ -420,12 +420,14 @@ function visitNode( * (because we want to wait until we are sure content to attach is available as a root) */ function detachPass( - delta: Delta.FieldChanges, + fieldChanges: Delta.FieldChanges, visitor: DeltaVisitor, config: PassConfig, + global?: readonly Delta.DetachedNodeChanges[], + rename?: readonly Delta.DetachedNodeRename[], ): void { - if (delta.global !== undefined) { - for (const { id, fields } of delta.global) { + if (global !== undefined) { + for (const { id, fields } of global) { let root = config.detachedFieldIndex.tryGetEntry(id); if (root === undefined) { const tree = tryGetFromNestedMap(config.refreshers, id.major, id.minor); @@ -439,33 +441,31 @@ function detachPass( config.attachPassRoots.set(root, fields); } } - if (delta.rename !== undefined) { - config.rootTransfers.push(...delta.rename); + if (rename !== undefined) { + config.rootTransfers.push(...rename); } - if (delta.local !== undefined) { - let index = 0; - for (const mark of delta.local) { - if (mark.fields !== undefined) { - assert( - mark.attach === undefined || mark.detach !== undefined, - 0x7d0 /* Invalid nested changes on an additive mark */, - ); - visitNode(index, mark.fields, visitor, config); - } - if (isDetachMark(mark)) { - for (let i = 0; i < mark.count; i += 1) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const id = offsetDetachId(mark.detach!, i); - const root = config.detachedFieldIndex.createEntry(id, config.latestRevision); - if (mark.fields !== undefined) { - config.attachPassRoots.set(root, mark.fields); - } - const field = config.detachedFieldIndex.toFieldKey(root); - visitor.detach({ start: index, end: index + 1 }, field); + let index = 0; + for (const mark of fieldChanges) { + if (mark.fields !== undefined) { + assert( + mark.attach === undefined || mark.detach !== undefined, + 0x7d0 /* Invalid nested changes on an additive mark */, + ); + visitNode(index, mark.fields, visitor, config); + } + if (isDetachMark(mark)) { + for (let i = 0; i < mark.count; i += 1) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const id = offsetDetachId(mark.detach!, i); + const root = config.detachedFieldIndex.createEntry(id, config.latestRevision); + if (mark.fields !== undefined) { + config.attachPassRoots.set(root, mark.fields); } - } else if (!isAttachMark(mark)) { - index += mark.count; + const field = config.detachedFieldIndex.toFieldKey(root); + visitor.detach({ start: index, end: index + 1 }, field); } + } else if (!isAttachMark(mark)) { + index += mark.count; } } } @@ -515,69 +515,67 @@ function collectDestroys( * - Collects detached roots (from replaces) that need an attach pass */ function attachPass( - delta: Delta.FieldChanges, + fieldChanges: Delta.FieldChanges, visitor: DeltaVisitor, config: PassConfig, ): void { - if (delta.local !== undefined) { - let index = 0; - for (const mark of delta.local) { - if (isAttachMark(mark) || isReplaceMark(mark)) { - for (let i = 0; i < mark.count; i += 1) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const offsetAttachId = offsetDetachId(mark.attach!, i); - let sourceRoot = config.detachedFieldIndex.tryGetEntry(offsetAttachId); - if (sourceRoot === undefined) { - const tree = tryGetFromNestedMap( - config.refreshers, - offsetAttachId.major, - offsetAttachId.minor, - ); - assert(tree !== undefined, 0x92a /* refresher data not found */); - buildTrees( - offsetAttachId, - [tree], - config.detachedFieldIndex, - config.latestRevision, - visitor, - ); - sourceRoot = config.detachedFieldIndex.getEntry(offsetAttachId); - } - const sourceField = config.detachedFieldIndex.toFieldKey(sourceRoot); - const offsetIndex = index + i; - if (isReplaceMark(mark)) { - const rootDestination = config.detachedFieldIndex.createEntry( - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - offsetDetachId(mark.detach!, i), - config.latestRevision, - ); - const destinationField = config.detachedFieldIndex.toFieldKey(rootDestination); - visitor.replace( - sourceField, - { start: offsetIndex, end: offsetIndex + 1 }, - destinationField, - ); - // We may need to do a second pass on the detached nodes - if (mark.fields !== undefined) { - config.attachPassRoots.set(rootDestination, mark.fields); - } - } else { - // This a simple attach - visitor.attach(sourceField, 1, offsetIndex); - } - config.detachedFieldIndex.deleteEntry(offsetAttachId); - const fields = config.attachPassRoots.get(sourceRoot); - if (fields !== undefined) { - config.attachPassRoots.delete(sourceRoot); - visitNode(offsetIndex, fields, visitor, config); + let index = 0; + for (const mark of fieldChanges) { + if (isAttachMark(mark) || isReplaceMark(mark)) { + for (let i = 0; i < mark.count; i += 1) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const offsetAttachId = offsetDetachId(mark.attach!, i); + let sourceRoot = config.detachedFieldIndex.tryGetEntry(offsetAttachId); + if (sourceRoot === undefined) { + const tree = tryGetFromNestedMap( + config.refreshers, + offsetAttachId.major, + offsetAttachId.minor, + ); + assert(tree !== undefined, 0x92a /* refresher data not found */); + buildTrees( + offsetAttachId, + [tree], + config.detachedFieldIndex, + config.latestRevision, + visitor, + ); + sourceRoot = config.detachedFieldIndex.getEntry(offsetAttachId); + } + const sourceField = config.detachedFieldIndex.toFieldKey(sourceRoot); + const offsetIndex = index + i; + if (isReplaceMark(mark)) { + const rootDestination = config.detachedFieldIndex.createEntry( + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + offsetDetachId(mark.detach!, i), + config.latestRevision, + ); + const destinationField = config.detachedFieldIndex.toFieldKey(rootDestination); + visitor.replace( + sourceField, + { start: offsetIndex, end: offsetIndex + 1 }, + destinationField, + ); + // We may need to do a second pass on the detached nodes + if (mark.fields !== undefined) { + config.attachPassRoots.set(rootDestination, mark.fields); } + } else { + // This a simple attach + visitor.attach(sourceField, 1, offsetIndex); + } + config.detachedFieldIndex.deleteEntry(offsetAttachId); + const fields = config.attachPassRoots.get(sourceRoot); + if (fields !== undefined) { + config.attachPassRoots.delete(sourceRoot); + visitNode(offsetIndex, fields, visitor, config); } - } else if (!isDetachMark(mark) && mark.fields !== undefined) { - visitNode(index, mark.fields, visitor, config); - } - if (!isDetachMark(mark)) { - index += mark.count; } + } else if (!isDetachMark(mark) && mark.fields !== undefined) { + visitNode(index, mark.fields, visitor, config); + } + if (!isDetachMark(mark)) { + index += mark.count; } } } diff --git a/packages/dds/tree/src/feature-libraries/forest-summary/forestSummarizer.ts b/packages/dds/tree/src/feature-libraries/forest-summary/forestSummarizer.ts index 52e11d40b567..c58a50ee12c8 100644 --- a/packages/dds/tree/src/feature-libraries/forest-summary/forestSummarizer.ts +++ b/packages/dds/tree/src/feature-libraries/forest-summary/forestSummarizer.ts @@ -150,12 +150,7 @@ export class ForestSummarizer implements Summarizable { id: buildId, trees: nodeCursors, }); - fieldChanges.push([ - fieldKey, - { - local: [{ count: nodeCursors.length, attach: buildId }], - }, - ]); + fieldChanges.push([fieldKey, [{ count: nodeCursors.length, attach: buildId }]]); } assert(this.forest.isEmpty, 0x797 /* forest must be empty */); From 09a1245d27eff5636ff009313d9d06ce2ae36e11 Mon Sep 17 00:00:00 2001 From: Jenn Date: Tue, 7 Jan 2025 17:57:39 +0000 Subject: [PATCH 03/16] avoid making into delta implementation functions have side effects --- .../default-schema/defaultFieldKinds.ts | 2 +- .../modular-schema/fieldChangeHandler.ts | 8 +++- .../modular-schema/genericFieldKind.ts | 12 ++++-- .../modular-schema/modularChangeFamily.ts | 40 ++++++++++++++----- .../optional-field/optionalField.ts | 19 ++++++--- 5 files changed, 59 insertions(+), 22 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts b/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts index 84f8d5dd91c9..3df21c05e3cc 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts @@ -43,7 +43,7 @@ export const noChangeHandler: FieldChangeHandler<0> = { }), codecsFactory: () => noChangeCodecFamily, editor: { buildChildChange: (index, change) => fail("Child changes not supported") }, - intoDelta: (change, deltaFromChild: ToDelta): DeltaFieldChanges => ({}), + intoDelta: (change, deltaFromChild: ToDelta) => [[], [], []], relevantRemovedRoots: (change): Iterable => [], isEmpty: (change: 0) => true, getNestedChanges: (change: 0) => [], diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts b/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts index 8237c9718672..6e153ef567eb 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts @@ -6,7 +6,9 @@ import type { ICodecFamily, IJsonCodec } from "../../codec/index.js"; import type { ChangeEncodingContext, + DeltaDetachedNodeChanges, DeltaDetachedNodeId, + DeltaDetachedNodeRename, DeltaFieldChanges, DeltaFieldMap, EncodedRevisionTag, @@ -49,7 +51,7 @@ export interface FieldChangeHandler< change: TChangeset, deltaFromChild: ToDelta, idAllocator: MemoizedIdRangeAllocator, - ): DeltaFieldChanges; + ): [DeltaFieldChanges, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]]; /** * Returns the set of removed roots that should be in memory for the given change to be applied. * A removed root is relevant if any of the following is true: @@ -195,7 +197,9 @@ export interface FieldEditor { * The `index` represents the index of the child node in the input context. * The `index` should be `undefined` iff the child node does not exist in the input context (e.g., an inserted node). */ -export type ToDelta = (child: NodeId) => DeltaFieldMap; +export type ToDelta = ( + child: NodeId, +) => [DeltaFieldMap, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]] | undefined; /** */ diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts b/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts index fbf5b1ad823b..981f26b6c731 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts @@ -5,7 +5,6 @@ import { type DeltaDetachedNodeId, - type DeltaFieldChanges, type DeltaMark, type RevisionMetadataSource, Multiplicity, @@ -47,7 +46,7 @@ export const genericChangeHandler: FieldChangeHandler = { return newGenericChangeset([[index, change]]); }, }, - intoDelta: (change: GenericChangeset, deltaFromChild: ToDelta): DeltaFieldChanges => { + intoDelta: (change: GenericChangeset, deltaFromChild: ToDelta) => { let nodeIndex = 0; const markList: DeltaMark[] = []; for (const [index, nodeChange] of change.entries()) { @@ -56,10 +55,15 @@ export const genericChangeHandler: FieldChangeHandler = { markList.push({ count: offset }); nodeIndex = index; } - markList.push({ count: 1, fields: deltaFromChild(nodeChange) }); + const childDelta = deltaFromChild(nodeChange); + if (childDelta !== undefined) { + // TODO accumulate the child global and rename + const [fields, _childGlobal, _childRename] = childDelta; + markList.push({ count: 1, fields }); + } nodeIndex += 1; } - return { local: markList }; + return [markList, [], []]; }, relevantRemovedRoots, isEmpty: (change: GenericChangeset): boolean => change.length === 0, diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts index da1103122496..57694b059d36 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts @@ -40,6 +40,8 @@ import { areEqualChangeAtomIdOpts, tagChange, makeAnonChange, + type DeltaDetachedNodeChanges, + type DeltaDetachedNodeRename, } from "../../core/index.js"; import { type IdAllocationState, @@ -1998,6 +2000,8 @@ export function updateRefreshers( } /** + * Converts a change into the delta format. + * * @param change - The change to convert into a delta. * @param fieldKinds - The field kinds to delegate to. */ @@ -2011,7 +2015,7 @@ export function intoDelta( if (!hasConflicts(change)) { // If there are no constraint violations, then tree changes apply. - const fieldDeltas = intoDeltaImpl( + const [fieldDeltas, global, rename] = intoDeltaImpl( change.fieldChanges, change.nodeChanges, idAllocator, @@ -2020,6 +2024,12 @@ export function intoDelta( if (fieldDeltas.size > 0) { rootDelta.fields = fieldDeltas; } + if (global.length > 0) { + rootDelta.global = global; + } + if (rename.length > 0) { + rootDelta.rename = rename; + } } // Constraint violations should not prevent nodes from being built @@ -2068,22 +2078,34 @@ function intoDeltaImpl( nodeChanges: ChangeAtomIdBTree, idAllocator: MemoizedIdRangeAllocator, fieldKinds: ReadonlyMap, -): Map { +): [Map, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]] { const delta: Map = new Map(); + const global: DeltaDetachedNodeChanges[] = []; + const rename: DeltaDetachedNodeRename[] = []; + for (const [field, fieldChange] of change) { - const deltaField = getChangeHandler(fieldKinds, fieldChange.fieldKind).intoDelta( + const [fieldChanges, fieldGlobal, fieldRename] = getChangeHandler( + fieldKinds, + fieldChange.fieldKind, + ).intoDelta( fieldChange.change, - (childChange): DeltaFieldMap => { + (childChange) => { const nodeChange = nodeChangeFromId(nodeChanges, childChange); return deltaFromNodeChange(nodeChange, nodeChanges, idAllocator, fieldKinds); }, idAllocator, ); - if (!isEmptyFieldChanges(deltaField)) { - delta.set(field, deltaField); + if (!isEmptyFieldChanges(fieldChanges)) { + delta.set(field, fieldChanges); + } + if (fieldGlobal.length > 0) { + fieldGlobal.forEach((c) => global.push(c)); + } + if (fieldRename.length > 0) { + fieldRename.forEach((r) => rename.push(r)); } } - return delta; + return [delta, global, rename]; } function deltaFromNodeChange( @@ -2091,12 +2113,12 @@ function deltaFromNodeChange( nodeChanges: ChangeAtomIdBTree, idAllocator: MemoizedIdRangeAllocator, fieldKinds: ReadonlyMap, -): DeltaFieldMap { +): [DeltaFieldMap, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]] | undefined { if (change.fieldChanges !== undefined) { return intoDeltaImpl(change.fieldChanges, nodeChanges, idAllocator, fieldKinds); } // TODO: update the API to allow undefined to be returned here - return new Map(); + return undefined; } /** diff --git a/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts b/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts index 408cbc6320c9..65c027602ed0 100644 --- a/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts +++ b/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts @@ -11,6 +11,7 @@ import { type ChangesetLocalId, type DeltaDetachedNodeChanges, type DeltaDetachedNodeId, + type DeltaDetachedNodeRename, type DeltaFieldChanges, type DeltaMark, type RevisionTag, @@ -657,8 +658,14 @@ export const optionalFieldEditor: OptionalFieldEditor = { export function optionalFieldIntoDelta( change: OptionalChangeset, deltaFromChild: ToDelta, -): DeltaFieldChanges { - const delta: Mutable = {}; +): { + fieldChanges: DeltaFieldChanges; + global?: DeltaDetachedNodeChanges[]; + rename?: DeltaDetachedNodeRename[]; +} { + let fieldChanges: DeltaFieldChanges = []; + let global: DeltaDetachedNodeChanges[] | undefined; + let rename: DeltaDetachedNodeRename[] | undefined; let markIsANoop = true; const mark: Mutable = { count: 1 }; @@ -674,7 +681,7 @@ export function optionalFieldIntoDelta( } if (change.moves.length > 0) { - delta.rename = change.moves.map(([src, dst]) => ({ + rename = change.moves.map(([src, dst]) => ({ count: 1, oldId: nodeIdFromChangeAtom(src), newId: nodeIdFromChangeAtom(dst), @@ -698,15 +705,15 @@ export function optionalFieldIntoDelta( } if (globals.length > 0) { - delta.global = globals; + global = globals; } } if (!markIsANoop) { - delta.local = [mark]; + fieldChanges = [mark]; } - return delta; + return { fieldChanges, global, rename }; } export const optionalChangeHandler: FieldChangeHandler< From 8097b8e7399749016b29ff7e27ece2ecf4749e1c Mon Sep 17 00:00:00 2001 From: Jenn Date: Tue, 7 Jan 2025 19:44:08 +0000 Subject: [PATCH 04/16] changes --- .../optional-field/optionalField.ts | 55 ++++++++++--------- .../sequence-field/sequenceFieldToDelta.ts | 49 ++++++++--------- packages/dds/tree/src/test/utils.ts | 14 ++++- 3 files changed, 63 insertions(+), 55 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts b/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts index 65c027602ed0..acbeb704e450 100644 --- a/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts +++ b/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts @@ -658,14 +658,10 @@ export const optionalFieldEditor: OptionalFieldEditor = { export function optionalFieldIntoDelta( change: OptionalChangeset, deltaFromChild: ToDelta, -): { - fieldChanges: DeltaFieldChanges; - global?: DeltaDetachedNodeChanges[]; - rename?: DeltaDetachedNodeRename[]; -} { +): [DeltaFieldChanges, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]] { let fieldChanges: DeltaFieldChanges = []; - let global: DeltaDetachedNodeChanges[] | undefined; - let rename: DeltaDetachedNodeRename[] | undefined; + const global: DeltaDetachedNodeChanges[] = []; + const rename: DeltaDetachedNodeRename[] = []; let markIsANoop = true; const mark: Mutable = { count: 1 }; @@ -681,39 +677,44 @@ export function optionalFieldIntoDelta( } if (change.moves.length > 0) { - rename = change.moves.map(([src, dst]) => ({ - count: 1, - oldId: nodeIdFromChangeAtom(src), - newId: nodeIdFromChangeAtom(dst), - })); + change.moves.forEach(([src, dst]) => + rename.push({ + count: 1, + oldId: nodeIdFromChangeAtom(src), + newId: nodeIdFromChangeAtom(dst), + }), + ); } if (change.childChanges.length > 0) { - const globals: DeltaDetachedNodeChanges[] = []; for (const [id, childChange] of change.childChanges) { const childDelta = deltaFromChild(childChange); - if (id !== "self") { - const fields = childDelta; - globals.push({ - id: { major: id.revision, minor: id.localId }, - fields, - }); - } else { - mark.fields = childDelta; - markIsANoop = false; + if (childDelta !== undefined) { + const [fields, childGlobal, childRename] = childDelta; + if (id !== "self") { + global.push({ + id: { major: id.revision, minor: id.localId }, + fields, + }); + } else { + mark.fields = fields; + markIsANoop = false; + } + if (childGlobal.length > 0) { + childGlobal.forEach((c) => global.push(c)); + } + if (childRename.length > 0) { + childRename.forEach((r) => rename.push(r)); + } } } - - if (globals.length > 0) { - global = globals; - } } if (!markIsANoop) { fieldChanges = [mark]; } - return { fieldChanges, global, rename }; + return [fieldChanges, global, rename]; } export const optionalChangeHandler: FieldChangeHandler< diff --git a/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts b/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts index 4198ed092080..cd0b92bd97ad 100644 --- a/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts +++ b/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts @@ -30,8 +30,8 @@ import type { ToDelta } from "../modular-schema/index.js"; export function sequenceFieldToDelta( change: MarkList, deltaFromChild: ToDelta, -): DeltaFieldChanges { - const local: DeltaMark[] = []; +): [DeltaFieldChanges, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]] { + const fieldChanges: DeltaMark[] = []; const global: DeltaDetachedNodeChanges[] = []; const rename: DeltaDetachedNodeRename[] = []; @@ -40,22 +40,29 @@ export function sequenceFieldToDelta( const inputCellId = getInputCellId(mark); const changes = mark.changes; if (changes !== undefined) { - const nestedDelta = deltaFromChild(changes); - if (nestedDelta.size > 0) { + const childDelta = deltaFromChild(changes); + if (childDelta !== undefined) { + const [fields, childGlobal, childRename] = childDelta; if (inputCellId === undefined) { - deltaMark.fields = nestedDelta; + deltaMark.fields = fields; } else { global.push({ id: nodeIdFromChangeAtom(inputCellId), - fields: nestedDelta, + fields, }); } + if (childGlobal.length > 0) { + childGlobal.forEach((c) => global.push(c)); + } + if (childRename.length > 0) { + childRename.forEach((r) => rename.push(r)); + } } } if (!areInputCellsEmpty(mark) && !areOutputCellsEmpty(mark)) { // Since each cell is associated with exactly one node, // the cell starting end ending populated means the cell content has not changed. - local.push(deltaMark); + fieldChanges.push(deltaMark); } else if (isAttachAndDetachEffect(mark)) { assert( inputCellId !== undefined, @@ -97,7 +104,7 @@ export function sequenceFieldToDelta( // Inline into `switch(mark.type)` once we upgrade to TS 4.7 switch (type) { case "MoveIn": { - local.push({ + fieldChanges.push({ attach: nodeIdFromChangeAtom(getEndpoint(mark)), count: mark.count, }); @@ -107,7 +114,7 @@ export function sequenceFieldToDelta( const newDetachId = getDetachedNodeId(mark); if (inputCellId === undefined) { deltaMark.detach = nodeIdFromChangeAtom(newDetachId); - local.push(deltaMark); + fieldChanges.push(deltaMark); } else { const oldId = nodeIdFromChangeAtom(inputCellId); // Removal of already removed content is only a no-op if the detach IDs are different. @@ -133,7 +140,7 @@ export function sequenceFieldToDelta( const detachId = nodeIdFromChangeAtom(getDetachedNodeId(mark)); if (inputCellId === undefined) { deltaMark.detach = detachId; - local.push(deltaMark); + fieldChanges.push(deltaMark); } else { // Move sources implicitly restore their content rename.push({ @@ -156,12 +163,12 @@ export function sequenceFieldToDelta( global.push({ id: buildId, fields: deltaMark.fields }); delete deltaMark.fields; } - local.push(deltaMark); + fieldChanges.push(deltaMark); break; } case NoopMarkType: if (inputCellId === undefined) { - local.push(deltaMark); + fieldChanges.push(deltaMark); } break; case "Rename": @@ -176,8 +183,8 @@ export function sequenceFieldToDelta( } } // Remove trailing no-op marks - while (hasSome(local)) { - const lastMark = getLast(local); + while (hasSome(fieldChanges)) { + const lastMark = getLast(fieldChanges); if ( lastMark.attach !== undefined || lastMark.detach !== undefined || @@ -185,17 +192,7 @@ export function sequenceFieldToDelta( ) { break; } - local.pop(); - } - const delta: Mutable = {}; - if (local.length > 0) { - delta.local = local; - } - if (global.length > 0) { - delta.global = global; - } - if (rename.length > 0) { - delta.rename = rename; + fieldChanges.pop(); } - return delta; + return [fieldChanges, global, rename]; } diff --git a/packages/dds/tree/src/test/utils.ts b/packages/dds/tree/src/test/utils.ts index 4967499a888b..e69d61e45f50 100644 --- a/packages/dds/tree/src/test/utils.ts +++ b/packages/dds/tree/src/test/utils.ts @@ -92,6 +92,8 @@ import { CursorLocationType, type RevertibleAlpha, type RevertibleAlphaFactory, + type DeltaDetachedNodeChanges, + type DeltaDetachedNodeRename, } from "../core/index.js"; import { typeboxValidator } from "../external-utilities/index.js"; import { @@ -460,8 +462,8 @@ export function spyOnMethod( /** * Determines whether or not the given delta has a visible impact on the document tree. */ -export function isDeltaVisible(delta: DeltaFieldChanges): boolean { - for (const mark of delta.local ?? []) { +export function isDeltaVisible(fieldChanges: DeltaFieldChanges): boolean { + for (const mark of fieldChanges ?? []) { if (mark.attach !== undefined || mark.detach !== undefined) { return true; } @@ -1042,10 +1044,18 @@ export function announceTestDelta( export function rootFromDeltaFieldMap( delta: DeltaFieldMap, + global?: readonly DeltaDetachedNodeChanges[], + rename?: readonly DeltaDetachedNodeRename[], build?: readonly DeltaDetachedNodeBuild[], destroy?: readonly DeltaDetachedNodeDestruction[], ): Mutable { const rootDelta: Mutable = { fields: delta }; + if (global !== undefined) { + rootDelta.global = global; + } + if (rename !== undefined) { + rootDelta.rename = rename; + } if (build !== undefined) { rootDelta.build = build; } From 3753faecd7492ae09c09be9dc1b0cae89a9f2d26 Mon Sep 17 00:00:00 2001 From: Jenn Date: Tue, 7 Jan 2025 22:51:28 +0000 Subject: [PATCH 05/16] remove change to ToDelta contract --- .../modular-schema/fieldChangeHandler.ts | 2 +- .../modular-schema/genericFieldKind.ts | 4 +--- .../modular-schema/modularChangeFamily.ts | 12 +++++++++++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts b/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts index 6e153ef567eb..7679c1f32060 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts @@ -199,7 +199,7 @@ export interface FieldEditor { */ export type ToDelta = ( child: NodeId, -) => [DeltaFieldMap, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]] | undefined; +) => DeltaFieldMap | undefined; /** */ diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts b/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts index 981f26b6c731..43d6568dc6cd 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts @@ -57,9 +57,7 @@ export const genericChangeHandler: FieldChangeHandler = { } const childDelta = deltaFromChild(nodeChange); if (childDelta !== undefined) { - // TODO accumulate the child global and rename - const [fields, _childGlobal, _childRename] = childDelta; - markList.push({ count: 1, fields }); + markList.push({ count: 1, fields: childDelta }); } nodeIndex += 1; } diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts index 57694b059d36..68353f729b49 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts @@ -2091,7 +2091,17 @@ function intoDeltaImpl( fieldChange.change, (childChange) => { const nodeChange = nodeChangeFromId(nodeChanges, childChange); - return deltaFromNodeChange(nodeChange, nodeChanges, idAllocator, fieldKinds); + const nodeChangeDelta = deltaFromNodeChange(nodeChange, nodeChanges, idAllocator, fieldKinds); + if (nodeChangeDelta !== undefined) { + const [nodeFieldChanges, nodeGlobals, nodeRenames] = nodeChangeDelta; + if (nodeGlobals.length > 0) { + nodeGlobals.forEach((c) => global.push(c)); + } + if (nodeRenames.length > 0) { + nodeRenames.forEach((r) => rename.push(r)); + } + return nodeFieldChanges; + } }, idAllocator, ); From 2daf01d54d41c8ef294a42a1d08f3aecca760f29 Mon Sep 17 00:00:00 2001 From: Jenn Date: Wed, 8 Jan 2025 00:22:04 +0000 Subject: [PATCH 06/16] fix field change handler into delta contract --- .../default-schema/defaultFieldKinds.ts | 2 +- .../modular-schema/fieldChangeHandler.ts | 24 +++++++++++++++---- .../modular-schema/genericFieldKind.ts | 2 +- .../feature-libraries/modular-schema/index.ts | 1 + .../modular-schema/modularChangeFamily.ts | 22 ++++++++++------- .../optional-field/optionalField.ts | 20 +++++++--------- .../sequence-field/sequenceFieldToDelta.ts | 22 +++++++---------- 7 files changed, 55 insertions(+), 38 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts b/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts index 3df21c05e3cc..ae6a6e26a010 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts @@ -43,7 +43,7 @@ export const noChangeHandler: FieldChangeHandler<0> = { }), codecsFactory: () => noChangeCodecFamily, editor: { buildChildChange: (index, change) => fail("Child changes not supported") }, - intoDelta: (change, deltaFromChild: ToDelta) => [[], [], []], + intoDelta: (change, deltaFromChild: ToDelta) => ({}), relevantRemovedRoots: (change): Iterable => [], isEmpty: (change: 0) => true, getNestedChanges: (change: 0) => [], diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts b/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts index 7679c1f32060..65fa2fe95ceb 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts @@ -28,6 +28,24 @@ export type NestedChangesIndices = [ number | undefined /* outputIndex */, ][]; +/** + * The return value of calling {@link FieldChangeHandler.intoDelta}. + */ +export interface FieldChangeDelta { + /** + * {@inheritdoc DeltaFieldChanges} + */ + readonly local?: DeltaFieldChanges; + /** + * {@inheritdoc DeltaRoot.global} + */ + readonly global?: readonly DeltaDetachedNodeChanges[]; + /** + * {@inheritdoc DeltaRoot.rename} + */ + readonly rename?: readonly DeltaDetachedNodeRename[]; +} + /** * Functionality provided by a field kind which will be composed with other `FieldChangeHandler`s to * implement a unified ChangeFamily supporting documents with multiple field kinds. @@ -51,7 +69,7 @@ export interface FieldChangeHandler< change: TChangeset, deltaFromChild: ToDelta, idAllocator: MemoizedIdRangeAllocator, - ): [DeltaFieldChanges, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]]; + ): FieldChangeDelta; /** * Returns the set of removed roots that should be in memory for the given change to be applied. * A removed root is relevant if any of the following is true: @@ -197,9 +215,7 @@ export interface FieldEditor { * The `index` represents the index of the child node in the input context. * The `index` should be `undefined` iff the child node does not exist in the input context (e.g., an inserted node). */ -export type ToDelta = ( - child: NodeId, -) => DeltaFieldMap | undefined; +export type ToDelta = (child: NodeId) => DeltaFieldMap | undefined; /** */ diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts b/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts index 43d6568dc6cd..6f44dfc678ab 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts @@ -61,7 +61,7 @@ export const genericChangeHandler: FieldChangeHandler = { } nodeIndex += 1; } - return [markList, [], []]; + return { local: markList }; }, relevantRemovedRoots, isEmpty: (change: GenericChangeset): boolean => change.length === 0, diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/index.ts b/packages/dds/tree/src/feature-libraries/modular-schema/index.ts index d829136aed98..035db01d7505 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/index.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/index.ts @@ -29,6 +29,7 @@ export { FlexFieldKind, type FullSchemaPolicy } from "./fieldKind.js"; export { FieldKindWithEditor } from "./fieldKindWithEditor.js"; export { type FieldChangeHandler, + type FieldChangeDelta, type FieldChangeRebaser, type FieldEditor, type NodeChangeComposer, diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts index 68353f729b49..36707ca23e94 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts @@ -2084,14 +2084,20 @@ function intoDeltaImpl( const rename: DeltaDetachedNodeRename[] = []; for (const [field, fieldChange] of change) { - const [fieldChanges, fieldGlobal, fieldRename] = getChangeHandler( - fieldKinds, - fieldChange.fieldKind, - ).intoDelta( + const { + local: fieldChanges, + global: fieldGlobal, + rename: fieldRename, + } = getChangeHandler(fieldKinds, fieldChange.fieldKind).intoDelta( fieldChange.change, (childChange) => { const nodeChange = nodeChangeFromId(nodeChanges, childChange); - const nodeChangeDelta = deltaFromNodeChange(nodeChange, nodeChanges, idAllocator, fieldKinds); + const nodeChangeDelta = deltaFromNodeChange( + nodeChange, + nodeChanges, + idAllocator, + fieldKinds, + ); if (nodeChangeDelta !== undefined) { const [nodeFieldChanges, nodeGlobals, nodeRenames] = nodeChangeDelta; if (nodeGlobals.length > 0) { @@ -2105,13 +2111,13 @@ function intoDeltaImpl( }, idAllocator, ); - if (!isEmptyFieldChanges(fieldChanges)) { + if (fieldChanges !== undefined && !isEmptyFieldChanges(fieldChanges)) { delta.set(field, fieldChanges); } - if (fieldGlobal.length > 0) { + if (fieldGlobal !== undefined) { fieldGlobal.forEach((c) => global.push(c)); } - if (fieldRename.length > 0) { + if (fieldRename !== undefined) { fieldRename.forEach((r) => rename.push(r)); } } diff --git a/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts b/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts index acbeb704e450..40a8d80f8bdb 100644 --- a/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts +++ b/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts @@ -41,6 +41,7 @@ import { type RelevantRemovedRootsFromChild, type ToDelta, type NestedChangesIndices, + type FieldChangeDelta, } from "../modular-schema/index.js"; import type { @@ -658,7 +659,7 @@ export const optionalFieldEditor: OptionalFieldEditor = { export function optionalFieldIntoDelta( change: OptionalChangeset, deltaFromChild: ToDelta, -): [DeltaFieldChanges, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]] { +): FieldChangeDelta { let fieldChanges: DeltaFieldChanges = []; const global: DeltaDetachedNodeChanges[] = []; const rename: DeltaDetachedNodeRename[] = []; @@ -690,22 +691,15 @@ export function optionalFieldIntoDelta( for (const [id, childChange] of change.childChanges) { const childDelta = deltaFromChild(childChange); if (childDelta !== undefined) { - const [fields, childGlobal, childRename] = childDelta; if (id !== "self") { global.push({ id: { major: id.revision, minor: id.localId }, - fields, + fields: childDelta, }); } else { - mark.fields = fields; + mark.fields = childDelta; markIsANoop = false; } - if (childGlobal.length > 0) { - childGlobal.forEach((c) => global.push(c)); - } - if (childRename.length > 0) { - childRename.forEach((r) => rename.push(r)); - } } } } @@ -714,7 +708,11 @@ export function optionalFieldIntoDelta( fieldChanges = [mark]; } - return [fieldChanges, global, rename]; + return { + local: fieldChanges, + global, + rename, + }; } export const optionalChangeHandler: FieldChangeHandler< diff --git a/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts b/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts index cd0b92bd97ad..379b46cb8e37 100644 --- a/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts +++ b/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts @@ -8,7 +8,6 @@ import { assert, unreachableCase } from "@fluidframework/core-utils/internal"; import { type DeltaDetachedNodeChanges, type DeltaDetachedNodeRename, - type DeltaFieldChanges, type DeltaMark, areEqualChangeAtomIds, } from "../../core/index.js"; @@ -25,12 +24,12 @@ import { getInputCellId, isAttachAndDetachEffect, } from "./utils.js"; -import type { ToDelta } from "../modular-schema/index.js"; +import type { FieldChangeDelta, ToDelta } from "../modular-schema/index.js"; export function sequenceFieldToDelta( change: MarkList, deltaFromChild: ToDelta, -): [DeltaFieldChanges, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]] { +): FieldChangeDelta { const fieldChanges: DeltaMark[] = []; const global: DeltaDetachedNodeChanges[] = []; const rename: DeltaDetachedNodeRename[] = []; @@ -42,21 +41,14 @@ export function sequenceFieldToDelta( if (changes !== undefined) { const childDelta = deltaFromChild(changes); if (childDelta !== undefined) { - const [fields, childGlobal, childRename] = childDelta; if (inputCellId === undefined) { - deltaMark.fields = fields; + deltaMark.fields = childDelta; } else { global.push({ id: nodeIdFromChangeAtom(inputCellId), - fields, + fields: childDelta, }); } - if (childGlobal.length > 0) { - childGlobal.forEach((c) => global.push(c)); - } - if (childRename.length > 0) { - childRename.forEach((r) => rename.push(r)); - } } } if (!areInputCellsEmpty(mark) && !areOutputCellsEmpty(mark)) { @@ -194,5 +186,9 @@ export function sequenceFieldToDelta( } fieldChanges.pop(); } - return [fieldChanges, global, rename]; + return { + local: fieldChanges, + global, + rename, + }; } From 7d65e5f7c1c8a55cc23e4d56243795d5938b7eb5 Mon Sep 17 00:00:00 2001 From: Jenn Date: Wed, 8 Jan 2025 21:30:34 +0000 Subject: [PATCH 07/16] revert field kind changes --- .../default-schema/defaultFieldKinds.ts | 3 +- .../optional-field/optionalField.ts | 48 +++++++++---------- .../sequence-field/sequenceFieldToDelta.ts | 44 +++++++++-------- 3 files changed, 49 insertions(+), 46 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts b/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts index ae6a6e26a010..30d434887a34 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts @@ -13,6 +13,7 @@ import { } from "../../core/index.js"; import { fail } from "../../util/index.js"; import { + type FieldChangeDelta, type FieldChangeHandler, type FieldEditor, type FieldKindConfiguration, @@ -43,7 +44,7 @@ export const noChangeHandler: FieldChangeHandler<0> = { }), codecsFactory: () => noChangeCodecFamily, editor: { buildChildChange: (index, change) => fail("Child changes not supported") }, - intoDelta: (change, deltaFromChild: ToDelta) => ({}), + intoDelta: (change, deltaFromChild: ToDelta): FieldChangeDelta => ({}), relevantRemovedRoots: (change): Iterable => [], isEmpty: (change: 0) => true, getNestedChanges: (change: 0) => [], diff --git a/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts b/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts index 40a8d80f8bdb..03858ef43238 100644 --- a/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts +++ b/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts @@ -660,9 +660,7 @@ export function optionalFieldIntoDelta( change: OptionalChangeset, deltaFromChild: ToDelta, ): FieldChangeDelta { - let fieldChanges: DeltaFieldChanges = []; - const global: DeltaDetachedNodeChanges[] = []; - const rename: DeltaDetachedNodeRename[] = []; + const delta: Mutable = {}; let markIsANoop = true; const mark: Mutable = { count: 1 }; @@ -678,41 +676,39 @@ export function optionalFieldIntoDelta( } if (change.moves.length > 0) { - change.moves.forEach(([src, dst]) => - rename.push({ - count: 1, - oldId: nodeIdFromChangeAtom(src), - newId: nodeIdFromChangeAtom(dst), - }), - ); + delta.rename = change.moves.map(([src, dst]) => ({ + count: 1, + oldId: nodeIdFromChangeAtom(src), + newId: nodeIdFromChangeAtom(dst), + })); } if (change.childChanges.length > 0) { + const globals: DeltaDetachedNodeChanges[] = []; for (const [id, childChange] of change.childChanges) { const childDelta = deltaFromChild(childChange); - if (childDelta !== undefined) { - if (id !== "self") { - global.push({ - id: { major: id.revision, minor: id.localId }, - fields: childDelta, - }); - } else { - mark.fields = childDelta; - markIsANoop = false; - } + if (id !== "self" && childDelta !== undefined) { + const fields = childDelta; + globals.push({ + id: { major: id.revision, minor: id.localId }, + fields, + }); + } else { + mark.fields = childDelta; + markIsANoop = false; } } + + if (globals.length > 0) { + delta.global = globals; + } } if (!markIsANoop) { - fieldChanges = [mark]; + delta.local = [mark]; } - return { - local: fieldChanges, - global, - rename, - }; + return delta; } export const optionalChangeHandler: FieldChangeHandler< diff --git a/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts b/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts index 379b46cb8e37..a33167225957 100644 --- a/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts +++ b/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts @@ -30,7 +30,7 @@ export function sequenceFieldToDelta( change: MarkList, deltaFromChild: ToDelta, ): FieldChangeDelta { - const fieldChanges: DeltaMark[] = []; + const local: DeltaMark[] = []; const global: DeltaDetachedNodeChanges[] = []; const rename: DeltaDetachedNodeRename[] = []; @@ -39,14 +39,14 @@ export function sequenceFieldToDelta( const inputCellId = getInputCellId(mark); const changes = mark.changes; if (changes !== undefined) { - const childDelta = deltaFromChild(changes); - if (childDelta !== undefined) { + const nestedDelta = deltaFromChild(changes); + if (nestedDelta !== undefined) { if (inputCellId === undefined) { - deltaMark.fields = childDelta; + deltaMark.fields = nestedDelta; } else { global.push({ id: nodeIdFromChangeAtom(inputCellId), - fields: childDelta, + fields: nestedDelta, }); } } @@ -54,7 +54,7 @@ export function sequenceFieldToDelta( if (!areInputCellsEmpty(mark) && !areOutputCellsEmpty(mark)) { // Since each cell is associated with exactly one node, // the cell starting end ending populated means the cell content has not changed. - fieldChanges.push(deltaMark); + local.push(deltaMark); } else if (isAttachAndDetachEffect(mark)) { assert( inputCellId !== undefined, @@ -96,7 +96,7 @@ export function sequenceFieldToDelta( // Inline into `switch(mark.type)` once we upgrade to TS 4.7 switch (type) { case "MoveIn": { - fieldChanges.push({ + local.push({ attach: nodeIdFromChangeAtom(getEndpoint(mark)), count: mark.count, }); @@ -106,7 +106,7 @@ export function sequenceFieldToDelta( const newDetachId = getDetachedNodeId(mark); if (inputCellId === undefined) { deltaMark.detach = nodeIdFromChangeAtom(newDetachId); - fieldChanges.push(deltaMark); + local.push(deltaMark); } else { const oldId = nodeIdFromChangeAtom(inputCellId); // Removal of already removed content is only a no-op if the detach IDs are different. @@ -132,7 +132,7 @@ export function sequenceFieldToDelta( const detachId = nodeIdFromChangeAtom(getDetachedNodeId(mark)); if (inputCellId === undefined) { deltaMark.detach = detachId; - fieldChanges.push(deltaMark); + local.push(deltaMark); } else { // Move sources implicitly restore their content rename.push({ @@ -155,12 +155,12 @@ export function sequenceFieldToDelta( global.push({ id: buildId, fields: deltaMark.fields }); delete deltaMark.fields; } - fieldChanges.push(deltaMark); + local.push(deltaMark); break; } case NoopMarkType: if (inputCellId === undefined) { - fieldChanges.push(deltaMark); + local.push(deltaMark); } break; case "Rename": @@ -175,8 +175,8 @@ export function sequenceFieldToDelta( } } // Remove trailing no-op marks - while (hasSome(fieldChanges)) { - const lastMark = getLast(fieldChanges); + while (hasSome(local)) { + const lastMark = getLast(local); if ( lastMark.attach !== undefined || lastMark.detach !== undefined || @@ -184,11 +184,17 @@ export function sequenceFieldToDelta( ) { break; } - fieldChanges.pop(); + local.pop(); } - return { - local: fieldChanges, - global, - rename, - }; + const delta: Mutable = {}; + if (local.length > 0) { + delta.local = local; + } + if (global.length > 0) { + delta.global = global; + } + if (rename.length > 0) { + delta.rename = rename; + } + return delta; } From 2d4ee09d4c266e1fe7a5017544fbf93fb1664ba5 Mon Sep 17 00:00:00 2001 From: Jenn Date: Wed, 8 Jan 2025 22:44:43 +0000 Subject: [PATCH 08/16] get build working --- .../default-schema/defaultFieldKinds.ts | 1 - .../dds/tree/src/feature-libraries/index.ts | 1 + .../optional-field/optionalField.ts | 2 - .../dds/tree/src/test/changesetWrapper.ts | 6 +- .../test/feature-libraries/deltaUtils.spec.ts | 64 ++-- .../modular-schema/basicRebasers.ts | 16 +- .../modular-schema/genericFieldKind.spec.ts | 12 +- .../modularChangeFamily.spec.ts | 28 +- .../modularChangeFamilyIntegration.spec.ts | 117 +++---- .../optionalChangeRebaser.test.ts | 14 +- .../optional-field/optionalField.spec.ts | 14 +- .../sequenceFieldToDelta.test.ts | 57 ++-- .../feature-libraries/sequence-field/utils.ts | 16 +- packages/dds/tree/src/test/forestTestSuite.ts | 299 ++++++++--------- packages/dds/tree/src/test/testChange.ts | 4 +- .../dds/tree/src/test/tree/anchorSet.spec.ts | 97 ++---- .../dds/tree/src/test/tree/visitDelta.spec.ts | 310 +++++++++--------- packages/dds/tree/src/test/utils.ts | 28 +- 18 files changed, 480 insertions(+), 606 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts b/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts index 30d434887a34..51c05b0bcca4 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts @@ -6,7 +6,6 @@ import { type ChangeAtomId, type DeltaDetachedNodeId, - type DeltaFieldChanges, type FieldKindIdentifier, forbiddenFieldKindIdentifier, Multiplicity, diff --git a/packages/dds/tree/src/feature-libraries/index.ts b/packages/dds/tree/src/feature-libraries/index.ts index d3055fb3214d..d042a9d2d58c 100644 --- a/packages/dds/tree/src/feature-libraries/index.ts +++ b/packages/dds/tree/src/feature-libraries/index.ts @@ -44,6 +44,7 @@ export { ModularEditBuilder, type FieldEditDescription as EditDescription, type FieldChangeHandler, + type FieldChangeDelta, type FieldChangeRebaser, type FieldEditor, type FieldChangeMap, diff --git a/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts b/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts index 03858ef43238..1500a7bea80f 100644 --- a/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts +++ b/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts @@ -11,8 +11,6 @@ import { type ChangesetLocalId, type DeltaDetachedNodeChanges, type DeltaDetachedNodeId, - type DeltaDetachedNodeRename, - type DeltaFieldChanges, type DeltaMark, type RevisionTag, areEqualChangeAtomIds, diff --git a/packages/dds/tree/src/test/changesetWrapper.ts b/packages/dds/tree/src/test/changesetWrapper.ts index 0d46959f52b9..1c43152985e6 100644 --- a/packages/dds/tree/src/test/changesetWrapper.ts +++ b/packages/dds/tree/src/test/changesetWrapper.ts @@ -7,7 +7,6 @@ import { strict } from "node:assert"; import { assert } from "@fluidframework/core-utils/internal"; import { type ChangeAtomIdMap, - type DeltaFieldChanges, type RevisionTag, type TaggedChange, makeAnonChange, @@ -16,6 +15,7 @@ import { taggedOptAtomId, } from "../core/index.js"; import type { + FieldChangeDelta, NodeChangeComposer, NodeChangePruner, NodeChangeRebaser, @@ -205,8 +205,8 @@ function prune( function toDelta( change: ChangesetWrapper, - fieldToDelta: (change: T, deltaFromChild: ToDelta) => DeltaFieldChanges, -): DeltaFieldChanges { + fieldToDelta: (change: T, deltaFromChild: ToDelta) => FieldChangeDelta, +): FieldChangeDelta { const deltaFromChild = (id: NodeId) => { const node = tryGetFromNestedMap(change.nodes, id.revision, id.localId); assert(node !== undefined, "Unknown node ID"); diff --git a/packages/dds/tree/src/test/feature-libraries/deltaUtils.spec.ts b/packages/dds/tree/src/test/feature-libraries/deltaUtils.spec.ts index 85e689d4e136..e62854c61c55 100644 --- a/packages/dds/tree/src/test/feature-libraries/deltaUtils.spec.ts +++ b/packages/dds/tree/src/test/feature-libraries/deltaUtils.spec.ts @@ -34,15 +34,13 @@ describe("DeltaUtils", () => { const nestedCursorInsert = new Map([ [ fooField, - { - local: [ - { count: 42 }, - { - count: 1, - attach: detachId, - }, - ], - }, + [ + { count: 42 }, + { + count: 1, + attach: detachId, + }, + ], ], ]); const input: DeltaRoot = { @@ -50,32 +48,28 @@ describe("DeltaUtils", () => { fields: new Map([ [ fooField, - { - local: [ - { - count: 1, - fields: nestedCursorInsert, - }, - ], - global: [{ id: detachId, fields: nestedCursorInsert }], - }, + [ + { + count: 1, + fields: nestedCursorInsert, + }, + ], ], ]), + global: [{ id: detachId, fields: nestedCursorInsert }], }; deepFreeze(input); const actual = mapRootChanges(input, mapTreeFromCursor); const nestedMapTreeInsert = new Map([ [ fooField, - { - local: [ - { count: 42 }, - { - count: 1, - attach: detachId, - }, - ], - }, + [ + { count: 42 }, + { + count: 1, + attach: detachId, + }, + ], ], ]); const expected: DeltaRoot = { @@ -83,17 +77,15 @@ describe("DeltaUtils", () => { fields: new Map([ [ fooField, - { - local: [ - { - count: 1, - fields: nestedMapTreeInsert, - }, - ], - global: [{ id: detachId, fields: nestedMapTreeInsert }], - }, + [ + { + count: 1, + fields: nestedMapTreeInsert, + }, + ], ], ]), + global: [{ id: detachId, fields: nestedMapTreeInsert }], }; deepFreeze(expected); assert.deepEqual(actual, expected); diff --git a/packages/dds/tree/src/test/feature-libraries/modular-schema/basicRebasers.ts b/packages/dds/tree/src/test/feature-libraries/modular-schema/basicRebasers.ts index 5fed15bc38c6..8bc3c0cf8e08 100644 --- a/packages/dds/tree/src/test/feature-libraries/modular-schema/basicRebasers.ts +++ b/packages/dds/tree/src/test/feature-libraries/modular-schema/basicRebasers.ts @@ -6,12 +6,9 @@ import { type TUnsafe, Type } from "@sinclair/typebox"; import { makeCodecFamily } from "../../../codec/index.js"; +import { makeDetachedNodeId, Multiplicity } from "../../../core/index.js"; import { - type DeltaFieldChanges, - makeDetachedNodeId, - Multiplicity, -} from "../../../core/index.js"; -import { + type FieldChangeDelta, type FieldChangeEncodingContext, type FieldChangeHandler, type FieldChangeRebaser, @@ -19,7 +16,7 @@ import { referenceFreeFieldChangeRebaser, // eslint-disable-next-line import/no-internal-modules } from "../../../feature-libraries/modular-schema/index.js"; -import { type Mutable, fail } from "../../../util/index.js"; +import { fail } from "../../../util/index.js"; import { makeValueCodec } from "../../codec/index.js"; /** @@ -85,16 +82,15 @@ export const valueHandler = { ]), editor: { buildChildChange: (index, change) => fail("Child changes not supported") }, - intoDelta: (change): DeltaFieldChanges => { - const delta: Mutable = {}; + intoDelta: (change): FieldChangeDelta => { if (change !== 0) { // We use the new and old numbers as the node ids. // These would have no real meaning to a delta consumer, but these delta are only used for testing. const detach = makeDetachedNodeId(undefined, change.old); const attach = makeDetachedNodeId(undefined, change.new); - delta.local = [{ count: 1, attach, detach }]; + return { local: [{ count: 1, attach, detach }] }; } - return delta; + return {}; }, relevantRemovedRoots: (change) => [], diff --git a/packages/dds/tree/src/test/feature-libraries/modular-schema/genericFieldKind.spec.ts b/packages/dds/tree/src/test/feature-libraries/modular-schema/genericFieldKind.spec.ts index dfbaa41d1167..92e4ee7aac23 100644 --- a/packages/dds/tree/src/test/feature-libraries/modular-schema/genericFieldKind.spec.ts +++ b/packages/dds/tree/src/test/feature-libraries/modular-schema/genericFieldKind.spec.ts @@ -189,13 +189,11 @@ describe("GenericField", () => { [2, nodeChange2], ]); - const expected: DeltaFieldChanges = { - local: [ - { count: 1, fields: TestNodeId.deltaFromChild(nodeChange1) }, - { count: 1 }, - { count: 1, fields: TestNodeId.deltaFromChild(nodeChange2) }, - ], - }; + const expected: DeltaFieldChanges = [ + { count: 1, fields: TestNodeId.deltaFromChild(nodeChange1) }, + { count: 1 }, + { count: 1, fields: TestNodeId.deltaFromChild(nodeChange2) }, + ]; const actual = genericChangeHandler.intoDelta( input, diff --git a/packages/dds/tree/src/test/feature-libraries/modular-schema/modularChangeFamily.spec.ts b/packages/dds/tree/src/test/feature-libraries/modular-schema/modularChangeFamily.spec.ts index a34ae17c3dff..e56a583466ab 100644 --- a/packages/dds/tree/src/test/feature-libraries/modular-schema/modularChangeFamily.spec.ts +++ b/packages/dds/tree/src/test/feature-libraries/modular-schema/modularChangeFamily.spec.ts @@ -92,6 +92,7 @@ import { } from "../../../feature-libraries/modular-schema/modularChangeFamily.js"; import type { EncodedNodeChangeset, + FieldChangeDelta, FieldChangeEncodingContext, // eslint-disable-next-line import/no-internal-modules } from "../../../feature-libraries/modular-schema/index.js"; @@ -139,7 +140,7 @@ const singleNodeHandler: FieldChangeHandler = { rebaser: singleNodeRebaser, codecsFactory: (revisionTagCodec) => makeCodecFamily([[1, singleNodeCodec]]), editor: singleNodeEditor, - intoDelta: (change, deltaFromChild): DeltaFieldChanges => ({ + intoDelta: (change, deltaFromChild): FieldChangeDelta => ({ local: [{ count: 1, fields: change !== undefined ? deltaFromChild(change) : undefined }], }), relevantRemovedRoots: (change, relevantRemovedRootsFromChild) => @@ -1047,26 +1048,19 @@ describe("ModularChangeFamily", () => { describe("intoDelta", () => { it("fieldChanges", () => { - const nodeDelta: DeltaFieldChanges = { - local: [ - { - count: 1, - fields: new Map([ - [ - fieldA, - { - local: [{ count: 1, detach: { minor: 0 }, attach: { minor: 1 } }], - }, - ], - ]), - }, - ], - }; + const nodeDelta: DeltaFieldChanges = [ + { + count: 1, + fields: new Map([ + [fieldA, [{ count: 1, detach: { minor: 0 }, attach: { minor: 1 } }]], + ]), + }, + ]; const expectedDelta: DeltaRoot = { fields: new Map([ [fieldA, nodeDelta], - [fieldB, { local: [{ count: 1, detach: { minor: 1 }, attach: { minor: 2 } }] }], + [fieldB, [{ count: 1, detach: { minor: 1 }, attach: { minor: 2 } }]], ]), }; diff --git a/packages/dds/tree/src/test/feature-libraries/modularChangeFamilyIntegration.spec.ts b/packages/dds/tree/src/test/feature-libraries/modularChangeFamilyIntegration.spec.ts index 5675014a60a6..6387a3a32d2c 100644 --- a/packages/dds/tree/src/test/feature-libraries/modularChangeFamilyIntegration.spec.ts +++ b/packages/dds/tree/src/test/feature-libraries/modularChangeFamilyIntegration.spec.ts @@ -566,30 +566,23 @@ describe("ModularChangeFamily integration", () => { const composedDelta = normalizeDelta(intoDelta(makeAnonChange(composed), fieldKinds)); const nodeAChanges: DeltaFieldMap = new Map([ - [fieldB, { local: [{ count: 1, attach: { minor: 1, major: tagForCompare } }] }], + [fieldB, [{ count: 1, attach: { minor: 1, major: tagForCompare } }]], ]); const nodeBChanges: DeltaFieldMap = new Map([ - [ - fieldC, - { - local: [{ count: 1, attach: { minor: 2, major: tagForCompare } }], - }, - ], + [fieldC, [{ count: 1, attach: { minor: 2, major: tagForCompare } }]], ]); const nodeCChanges: DeltaFieldMap = new Map([ - [fieldC, { local: [{ count: 1, detach: { minor: 3, major: tagForCompare } }] }], + [fieldC, [{ count: 1, detach: { minor: 3, major: tagForCompare } }]], ]); - const fieldAChanges: DeltaFieldChanges = { - local: [ - { count: 1, detach: { minor: 0, major: tagForCompare }, fields: nodeAChanges }, - { count: 1, attach: { minor: 0, major: tagForCompare } }, - { count: 1, detach: { minor: 1, major: tagForCompare }, fields: nodeBChanges }, - { count: 1, detach: { minor: 2, major: tagForCompare }, fields: nodeCChanges }, - ], - }; + const fieldAChanges: DeltaFieldChanges = [ + { count: 1, detach: { minor: 0, major: tagForCompare }, fields: nodeAChanges }, + { count: 1, attach: { minor: 0, major: tagForCompare } }, + { count: 1, detach: { minor: 1, major: tagForCompare }, fields: nodeBChanges }, + { count: 1, detach: { minor: 2, major: tagForCompare }, fields: nodeCChanges }, + ]; const expectedDelta: DeltaRoot = normalizeDelta({ fields: new Map([[fieldA, fieldAChanges]]), @@ -630,29 +623,17 @@ describe("ModularChangeFamily integration", () => { fields: new Map([ [ fieldA, - { - local: [ - { - count: 1, - detach: { minor: 0, major: tagForCompare }, - fields: new Map([ - [ - fieldC, - { - local: [{ count: 1, attach: { minor: 2, major: tagForCompare } }], - }, - ], - ]), - }, - ], - }, - ], - [ - fieldB, - { - local: [{ count: 1, attach: { minor: 0, major: tagForCompare } }], - }, + [ + { + count: 1, + detach: { minor: 0, major: tagForCompare }, + fields: new Map([ + [fieldC, [{ count: 1, attach: { minor: 2, major: tagForCompare } }]], + ]), + }, + ], ], + [fieldB, [{ count: 1, attach: { minor: 0, major: tagForCompare } }]], ]), }; @@ -707,22 +688,13 @@ describe("ModularChangeFamily integration", () => { fields: new Map([ [ fieldB, - { - local: [ - { count: 1 }, - { - count: 1, - fields: new Map([ - [ - fieldC, - { - local: [{ count: 1, attach: { major: tag2, minor: 2 } }], - }, - ], - ]), - }, - ], - }, + [ + { count: 1 }, + { + count: 1, + fields: new Map([[fieldC, [{ count: 1, attach: { major: tag2, minor: 2 } }]]]), + }, + ], ], ]), }; @@ -968,8 +940,8 @@ describe("ModularChangeFamily integration", () => { }; const expected: DeltaRoot = { fields: new Map([ - [brand("foo"), { local: [moveOut1, moveIn1] }], - [brand("bar"), { local: [moveOut2, moveIn2] }], + [brand("foo"), [moveOut1, moveIn1]], + [brand("bar"), [moveOut2, moveIn2]], ]), }; const actual = intoDelta(makeAnonChange(change), family.fieldKinds); @@ -996,6 +968,19 @@ function normalizeDelta( trees, })); } + if (delta.global !== undefined && delta.global.length > 0) { + normalized.global = delta.global.map(({ id, fields }) => ({ + id: normalizeDeltaDetachedNodeId(id, genId, map), + fields: normalizeDeltaFieldMap(fields, genId, map), + })); + } + if (delta.rename !== undefined && delta.rename.length > 0) { + normalized.rename = delta.rename.map(({ oldId, count, newId }) => ({ + oldId: normalizeDeltaDetachedNodeId(oldId, genId, map), + count, + newId: normalizeDeltaDetachedNodeId(newId, genId, map), + })); + } return normalized; } @@ -1017,25 +1002,11 @@ function normalizeDeltaFieldChanges( genId: IdAllocator, idMap: Map, ): DeltaFieldChanges { - const normalized: Mutable = {}; - if (delta.local !== undefined && delta.local.length > 0) { - normalized.local = delta.local.map((mark) => normalizeDeltaMark(mark, genId, idMap)); - } - if (delta.global !== undefined && delta.global.length > 0) { - normalized.global = delta.global.map(({ id, fields }) => ({ - id: normalizeDeltaDetachedNodeId(id, genId, idMap), - fields: normalizeDeltaFieldMap(fields, genId, idMap), - })); - } - if (delta.rename !== undefined && delta.rename.length > 0) { - normalized.rename = delta.rename.map(({ oldId, count, newId }) => ({ - oldId: normalizeDeltaDetachedNodeId(oldId, genId, idMap), - count, - newId: normalizeDeltaDetachedNodeId(newId, genId, idMap), - })); + if (delta.length > 0) { + return delta.map((mark) => normalizeDeltaMark(mark, genId, idMap)); } - return normalized; + return delta; } function normalizeDeltaMark( diff --git a/packages/dds/tree/src/test/feature-libraries/optional-field/optionalChangeRebaser.test.ts b/packages/dds/tree/src/test/feature-libraries/optional-field/optionalChangeRebaser.test.ts index 13189bf7e58b..3286a956810c 100644 --- a/packages/dds/tree/src/test/feature-libraries/optional-field/optionalChangeRebaser.test.ts +++ b/packages/dds/tree/src/test/feature-libraries/optional-field/optionalChangeRebaser.test.ts @@ -6,12 +6,11 @@ import { strict as assert } from "node:assert"; import { describeStress, StressMode } from "@fluid-private/stochastic-test-utils"; -import type { CrossFieldManager } from "../../../feature-libraries/index.js"; +import type { CrossFieldManager, FieldChangeDelta } from "../../../feature-libraries/index.js"; import { type ChangeAtomId, type ChangeAtomIdMap, type ChangesetLocalId, - type DeltaFieldChanges, type RevisionMetadataSource, type RevisionTag, type TaggedChange, @@ -108,7 +107,7 @@ const failCrossFieldManager: CrossFieldManager = { function toDelta( change: OptionalChangeset, deltaFromChild: ToDelta = TestNodeId.deltaFromChild, -): DeltaFieldChanges { +): FieldChangeDelta { return optionalFieldIntoDelta(change, deltaFromChild); } @@ -266,7 +265,8 @@ function composeWrapped( } function isWrappedChangeEmpty(change: WrappedChangeset): boolean { - return !isDeltaVisible(toDeltaWrapped(makeAnonChange(change))); + const delta = toDeltaWrapped(makeAnonChange(change)).local; + return delta === undefined || !isDeltaVisible(delta); } function assertWrappedChangesetsEquivalent( @@ -525,7 +525,8 @@ function runSingleEditRebaseAxiomSuite(initialState: OptionalFieldTestState) { it(`${name} ○ ${name}⁻¹ === ε`, () => { const inv = invertWrapped(change, tag1, true); const actual = composeWrapped(change, tagRollbackInverse(inv, tag1, change.revision)); - const delta = toDeltaWrapped(makeAnonChange(actual)); + const delta = toDeltaWrapped(makeAnonChange(actual)).local; + assert(delta !== undefined); assert.equal(isDeltaVisible(delta), false); }); } @@ -540,7 +541,8 @@ function runSingleEditRebaseAxiomSuite(initialState: OptionalFieldTestState) { change.revision, ); const actual = composeWrapped(inv, change); - const delta = toDeltaWrapped(makeAnonChange(actual)); + const delta = toDeltaWrapped(makeAnonChange(actual)).local; + assert(delta !== undefined); assert.equal(isDeltaVisible(delta), false); }); } diff --git a/packages/dds/tree/src/test/feature-libraries/optional-field/optionalField.spec.ts b/packages/dds/tree/src/test/feature-libraries/optional-field/optionalField.spec.ts index 7c573f81fdc3..600be056cdd3 100644 --- a/packages/dds/tree/src/test/feature-libraries/optional-field/optionalField.spec.ts +++ b/packages/dds/tree/src/test/feature-libraries/optional-field/optionalField.spec.ts @@ -7,7 +7,6 @@ import { strict as assert, fail } from "node:assert"; import { type ChangeAtomId, - type DeltaFieldChanges, type TaggedChange, makeAnonChange, makeDetachedNodeId, @@ -43,8 +42,11 @@ import { testRebaserAxioms } from "./optionalChangeRebaser.test.js"; import { testCodecs } from "./optionalFieldChangeCodecs.test.js"; import { deepFreeze } from "@fluidframework/test-runtime-utils/internal"; import { testReplaceRevisions } from "./replaceRevisions.test.js"; -// eslint-disable-next-line import/no-internal-modules -import type { NestedChangesIndices } from "../../../feature-libraries/modular-schema/fieldChangeHandler.js"; +import type { + FieldChangeDelta, + NestedChangesIndices, + // eslint-disable-next-line import/no-internal-modules +} from "../../../feature-libraries/modular-schema/fieldChangeHandler.js"; /** * A change to a child encoding as a simple placeholder string. @@ -671,7 +673,7 @@ describe("optionalField", () => { describe("IntoDelta", () => { it("can be converted to a delta when field was empty", () => { const outerNodeId = makeDetachedNodeId(tag, 41); - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { global: [ { id: outerNodeId, @@ -686,7 +688,7 @@ describe("optionalField", () => { }); it("can be converted to a delta when restoring content", () => { - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { local: [ { count: 1, @@ -701,7 +703,7 @@ describe("optionalField", () => { }); it("can be converted to a delta with only child changes", () => { - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { local: [ { count: 1, diff --git a/packages/dds/tree/src/test/feature-libraries/sequence-field/sequenceFieldToDelta.test.ts b/packages/dds/tree/src/test/feature-libraries/sequence-field/sequenceFieldToDelta.test.ts index ca66ea2b5255..b1a3be331d9a 100644 --- a/packages/dds/tree/src/test/feature-libraries/sequence-field/sequenceFieldToDelta.test.ts +++ b/packages/dds/tree/src/test/feature-libraries/sequence-field/sequenceFieldToDelta.test.ts @@ -8,7 +8,6 @@ import { strict as assert, fail } from "node:assert"; import { type ChangesetLocalId, type DeltaDetachedNodeId, - type DeltaFieldChanges, type DeltaFieldMap, type DeltaMark, type FieldKey, @@ -16,7 +15,11 @@ import { emptyFieldChanges, tagChange, } from "../../../core/index.js"; -import { type NodeId, SequenceField as SF } from "../../../feature-libraries/index.js"; +import { + type FieldChangeDelta, + type NodeId, + SequenceField as SF, +} from "../../../feature-libraries/index.js"; import { brand } from "../../../util/index.js"; import { TestChange } from "../../testChange.js"; import { assertFieldChangesEqual, mintRevisionTag } from "../../utils.js"; @@ -34,7 +37,7 @@ const fooField = brand("foo"); const cellId = { revision: tag1, localId: brand(0) }; const deltaNodeId: DeltaDetachedNodeId = { major: cellId.revision, minor: cellId.localId }; -function toDeltaShallow(change: SF.Changeset): DeltaFieldChanges { +function toDeltaShallow(change: SF.Changeset): FieldChangeDelta { deepFreeze(change); return SF.sequenceFieldToDelta(change, () => fail("Unexpected call to child ToDelta")); } @@ -54,14 +57,14 @@ export function testToDelta() { it("child change", () => { const actual = toDelta(inlineRevision(Change.modify(0, childChange1), tag)); const markList: DeltaMark[] = [{ count: 1, fields: childChange1Delta }]; - const expected: DeltaFieldChanges = { local: markList }; + const expected: FieldChangeDelta = { local: markList }; assert.deepEqual(actual, expected); }); it("child change under removed node", () => { const modify = [Mark.modify(childChange1, { revision: tag, localId: brand(42) })]; const actual = toDelta(inlineRevision(modify, tag)); - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { global: [{ id: detachId, fields: childChange1Delta }], }; assertFieldChangesEqual(actual, expected); @@ -86,7 +89,7 @@ export function testToDelta() { it("revive => restore", () => { const changeset = Change.revive(0, 1, { revision: tag, localId: brand(0) }, tag2); const actual = toDelta(changeset); - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { local: [ { count: 1, @@ -102,13 +105,13 @@ export function testToDelta() { const changeset = [ Mark.revive(1, { revision: tag, localId: brand(0) }, { changes: nodeId }), ]; - const fieldChanges = new Map([[fooField, {}]]); + const fieldChanges = new Map([[fooField, []]]); const deltaFromChild = (child: NodeId): DeltaFieldMap => { assert.deepEqual(child, nodeId); return fieldChanges; }; const actual = SF.sequenceFieldToDelta(changeset, deltaFromChild); - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { local: [ { count: 1, @@ -127,7 +130,7 @@ export function testToDelta() { it("remove", () => { const changeset = [Mark.remove(10, brand(42))]; - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { local: [ { count: 10, @@ -142,7 +145,7 @@ export function testToDelta() { it("remove with override", () => { const detachIdOverride: SF.CellId = { revision: tag2, localId: brand(1) }; const changeset = [Mark.remove(10, brand(42), { idOverride: detachIdOverride })]; - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { local: [ { count: 10, @@ -170,7 +173,7 @@ export function testToDelta() { count: 10, }; const markList: DeltaMark[] = [{ count: 42 }, moveOut, { count: 8 }, moveIn]; - const expected: DeltaFieldChanges = { local: markList }; + const expected: FieldChangeDelta = { local: markList }; const actual = toDelta(changeset); assert.deepStrictEqual(actual, expected); }); @@ -209,7 +212,7 @@ export function testToDelta() { count: 3, }; const markList: DeltaMark[] = [moveOut1, moveIn1, moveOut2, moveIn2, moveOut3, moveIn3]; - const expected: DeltaFieldChanges = { local: markList }; + const expected: FieldChangeDelta = { local: markList }; const actual = toDelta(changeset); assert.deepStrictEqual(actual, expected); }); @@ -237,7 +240,7 @@ export function testToDelta() { { count: 1 }, { count: 1, fields: childChange1Delta }, ]; - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { local: markList, }; const actual = toDelta(inlineRevision(changeset, tag)); @@ -247,7 +250,7 @@ export function testToDelta() { it("insert and modify => insert", () => { const changeset = [Mark.insert(1, brand(0), { changes: childChange1 })]; const buildId = { major: tag, minor: 0 }; - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { global: [{ id: buildId, fields: childChange1Delta }], local: [{ count: 1, attach: buildId }], }; @@ -257,7 +260,7 @@ export function testToDelta() { it("modify and remove => remove", () => { const changeset = [Mark.remove(1, brand(42), { changes: childChange1 })]; - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { local: [{ count: 1, detach: detachId, fields: childChange1Delta }], }; const actual = toDelta(inlineRevision(changeset, tag)); @@ -266,7 +269,7 @@ export function testToDelta() { it("modify and move-out => move-out", () => { const changeset = [Mark.moveOut(1, moveId, { changes: childChange1 })]; - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { local: [ { count: 1, detach: { major: tag, minor: moveId }, fields: childChange1Delta }, ], @@ -280,10 +283,10 @@ export function testToDelta() { const changeset = [Mark.insert(1, brand(0), { changes: nodeId })]; const nestedMoveDelta = new Map([ - [fooField, { local: [{ attach: { minor: moveId }, count: 42 }] }], + [fooField, [{ attach: { minor: moveId }, count: 42 }]], ]); const buildId = { minor: 0 }; - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { global: [{ id: buildId, fields: nestedMoveDelta }], local: [{ count: 1, attach: buildId }], }; @@ -301,7 +304,7 @@ export function testToDelta() { const changeset = [Mark.remove(2, brand(2), { cellId: { localId: brand(0) } })]; const delta = toDelta(changeset); const buildId = { minor: 0 }; - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { rename: [{ count: 2, oldId: buildId, newId: { minor: 2 } }], }; assertFieldChangesEqual(delta, expected); @@ -317,7 +320,7 @@ export function testToDelta() { const delta = toDelta(changeset); const buildId = { minor: 0 }; const id = { minor: 2 }; - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { rename: [{ oldId: buildId, newId: id, count: 2 }], local: [{ count: 1 }, { count: 2, attach: id }], }; @@ -334,7 +337,7 @@ export function testToDelta() { const delta = toDelta(changeset); const id = { minor: 0 }; - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { local: [{ count: 2, detach: id }], rename: [{ count: 2, oldId: id, newId: { minor: 4 } }], }; @@ -352,7 +355,7 @@ export function testToDelta() { const buildId = { minor: 0 }; const id1 = { minor: 2 }; const id2 = { minor: 6 }; - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { rename: [ { count: 2, oldId: buildId, newId: id1 }, { count: 2, oldId: id1, newId: id2 }, @@ -373,7 +376,7 @@ export function testToDelta() { const delta = toDelta(changeset); const id = { minor: 0 }; - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { local: [ { count: 2, detach: id }, { count: 1 }, @@ -393,7 +396,7 @@ export function testToDelta() { ]; const actual = toDelta(move); - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { rename: [{ count: 1, oldId: deltaNodeId, newId: { minor: 0 } }], local: [{ count: 1, attach: { minor: 0 } }], }; @@ -404,7 +407,7 @@ export function testToDelta() { it("remove", () => { const deletion = [Mark.remove(1, brand(0), { cellId })]; const actual = toDelta(inlineRevision(deletion, tag)); - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { rename: [ { count: 1, @@ -419,7 +422,7 @@ export function testToDelta() { it("modify and remove", () => { const deletion = [Mark.remove(1, brand(0), { cellId, changes: childChange1 })]; const actual = toDelta(inlineRevision(deletion, tag)); - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { rename: [ { count: 1, @@ -443,7 +446,7 @@ export function testToDelta() { Mark.pin(1, brand(1), { changes: childChange1 }), ]; const actual = toDelta(inlineRevision(changeset, tag)); - const expected: DeltaFieldChanges = { + const expected: FieldChangeDelta = { local: [{ count: 1 }, { count: 1, fields: childChange1Delta }], }; assertFieldChangesEqual(actual, expected); diff --git a/packages/dds/tree/src/test/feature-libraries/sequence-field/utils.ts b/packages/dds/tree/src/test/feature-libraries/sequence-field/utils.ts index 71d22464d3e1..cf7d15617d21 100644 --- a/packages/dds/tree/src/test/feature-libraries/sequence-field/utils.ts +++ b/packages/dds/tree/src/test/feature-libraries/sequence-field/utils.ts @@ -12,7 +12,7 @@ import { type ChangeAtomId, type ChangeAtomIdMap, type ChangesetLocalId, - type DeltaFieldChanges, + type DeltaRoot, type RevisionInfo, type RevisionMetadataSource, type RevisionTag, @@ -20,6 +20,7 @@ import { makeAnonChange, mapTaggedChange, revisionMetadataSourceFromInfo, + rootFieldKey, tagChange, tagRollbackInverse, } from "../../../core/index.js"; @@ -74,7 +75,7 @@ import { tryGetFromNestedMap, } from "../../../util/index.js"; import { - assertFieldChangesEqual, + assertDeltaEqual, assertIsSessionId, defaultRevInfosFromChanges, defaultRevisionMetadataFromChanges, @@ -490,12 +491,17 @@ export function invert( } export function checkDeltaEquality(actual: SF.Changeset, expected: SF.Changeset) { - assertFieldChangesEqual(toDelta(actual), toDelta(expected)); + assertDeltaEqual(toDelta(actual), toDelta(expected)); } -export function toDelta(change: SF.Changeset): DeltaFieldChanges { +export function toDelta(change: SF.Changeset): DeltaRoot { deepFreeze(change); - return SF.sequenceFieldToDelta(change, TestNodeId.deltaFromChild); + const { local, global, rename } = SF.sequenceFieldToDelta(change, TestNodeId.deltaFromChild); + return { + fields: local === undefined ? new Map([]) : new Map([[rootFieldKey, local]]), + global, + rename, + }; } export function toDeltaWrapped(change: WrappedChange) { diff --git a/packages/dds/tree/src/test/forestTestSuite.ts b/packages/dds/tree/src/test/forestTestSuite.ts index f36ca568f500..9cfbcfe39e22 100644 --- a/packages/dds/tree/src/test/forestTestSuite.ts +++ b/packages/dds/tree/src/test/forestTestSuite.ts @@ -180,16 +180,10 @@ export function testForest(config: ForestTestConfiguration): void { const forest = factory(new TreeStoredSchemaRepository(toStoredSchema(JsonArray))); assert(forest.isEmpty); - const insert: DeltaFieldChanges = { - local: [{ count: 1, attach: { minor: 1 } }], - }; - applyTestDelta( - new Map([[brand("different root"), insert]]), - forest, - undefined, - undefined, - [{ id: { minor: 1 }, trees: [singleJsonCursor([])] }], - ); + const insert: DeltaFieldChanges = [{ count: 1, attach: { minor: 1 } }]; + applyTestDelta(new Map([[brand("different root"), insert]]), forest, { + build: [{ id: { minor: 1 }, trees: [singleJsonCursor([])] }], + }); assert(!forest.isEmpty); }); @@ -411,13 +405,9 @@ export function testForest(config: ForestTestConfiguration): void { cursor.clear(); const mark: DeltaMark = { count: 1, detach: detachId }; - const delta: DeltaFieldMap = new Map([[rootFieldKey, { local: [mark] }]]); - applyTestDelta(delta, forest, undefined, undefined, undefined, [ - { id: detachId, count: 1 }, - ]); - applyTestDelta(delta, forest.anchors, undefined, undefined, undefined, [ - { id: detachId, count: 1 }, - ]); + const delta: DeltaFieldMap = new Map([[rootFieldKey, [mark]]]); + applyTestDelta(delta, forest, { destroy: [{ id: detachId, count: 1 }] }); + applyTestDelta(delta, forest.anchors, { destroy: [{ id: detachId, count: 1 }] }); assert.equal( forest.tryMoveCursorToNode(firstNodeAnchor, cursor), @@ -447,9 +437,9 @@ export function testForest(config: ForestTestConfiguration): void { { jsonValidator: typeboxValidator }, ); const delta: DeltaFieldMap = new Map([ - [rootFieldKey, { local: [mark] }], + [rootFieldKey, [mark]], ]); - applyTestDelta(delta, forest, detachedFieldIndex); + applyTestDelta(delta, forest, { detachedFieldIndex }); const detachedField: DetachedField = brand( detachedFieldIndex.toFieldKey(0 as ForestRootId), @@ -563,7 +553,7 @@ export function testForest(config: ForestTestConfiguration): void { const clone = forest.clone(schema, forest.anchors); const mark: DeltaMark = { count: 1, detach: detachId }; - const delta: DeltaFieldMap = new Map([[rootFieldKey, { local: [mark] }]]); + const delta: DeltaFieldMap = new Map([[rootFieldKey, [mark]]]); applyTestDelta(delta, clone); // Check the clone has the new value @@ -593,7 +583,7 @@ export function testForest(config: ForestTestConfiguration): void { moveToDetachedField(forest, cursor); const mark: DeltaMark = { count: 1, detach: detachId }; - const delta: DeltaFieldMap = new Map([[rootFieldKey, { local: [mark] }]]); + const delta: DeltaFieldMap = new Map([[rootFieldKey, [mark]]]); assert.throws(() => applyTestDelta(delta, forest)); }); @@ -614,7 +604,7 @@ export function testForest(config: ForestTestConfiguration): void { }); const mark: DeltaMark = { count: 1, detach: detachId }; - const delta: DeltaFieldMap = new Map([[rootFieldKey, { local: [mark] }]]); + const delta: DeltaFieldMap = new Map([[rootFieldKey, [mark]]]); assert.throws(() => applyTestDelta(delta, forest)); assert.deepEqual(log, ["beforeChange"]); }); @@ -635,7 +625,7 @@ export function testForest(config: ForestTestConfiguration): void { }); const mark: DeltaMark = { count: 1, detach: detachId }; - const delta: DeltaFieldMap = new Map([[rootFieldKey, { local: [mark] }]]); + const delta: DeltaFieldMap = new Map([[rootFieldKey, [mark]]]); applyTestDelta(delta, forest); assert.deepEqual(log, ["beforeChange"]); }); @@ -651,27 +641,22 @@ export function testForest(config: ForestTestConfiguration): void { const setField: DeltaMark = { count: 1, - fields: new Map([ - [ - xField, - { - local: [{ count: 1, detach: detachId, attach: buildId }], - }, - ], - ]), + fields: new Map([[xField, [{ count: 1, detach: detachId, attach: buildId }]]]), }; - const delta: DeltaFieldMap = new Map([[rootFieldKey, { local: [setField] }]]); - applyTestDelta(delta, forest, undefined, undefined, [ - { - id: buildId, - trees: [ - cursorForJsonableTreeNode({ - type: brand(booleanSchema.identifier), - value: true, - }), - ], - }, - ]); + const delta: DeltaFieldMap = new Map([[rootFieldKey, [setField]]]); + applyTestDelta(delta, forest, { + build: [ + { + id: buildId, + trees: [ + cursorForJsonableTreeNode({ + type: brand(booleanSchema.identifier), + value: true, + }), + ], + }, + ], + }); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -692,22 +677,22 @@ export function testForest(config: ForestTestConfiguration): void { const setField: DeltaMark = { count: 1, - fields: new Map([ - [xField, { local: [{ count: 1, detach: detachId, attach: buildId }] }], - ]), + fields: new Map([[xField, [{ count: 1, detach: detachId, attach: buildId }]]]), }; - const delta: DeltaFieldMap = new Map([[rootFieldKey, { local: [setField] }]]); - applyTestDelta(delta, forest, undefined, undefined, [ - { - id: buildId, - trees: [ - cursorForJsonableTreeNode({ - type: brand(booleanSchema.identifier), - value: true, - }), - ], - }, - ]); + const delta: DeltaFieldMap = new Map([[rootFieldKey, [setField]]]); + applyTestDelta(delta, forest, { + build: [ + { + id: buildId, + trees: [ + cursorForJsonableTreeNode({ + type: brand(booleanSchema.identifier), + value: true, + }), + ], + }, + ], + }); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -731,7 +716,7 @@ export function testForest(config: ForestTestConfiguration): void { count: 1, detach: detachId, }; - const delta: DeltaFieldMap = new Map([[rootFieldKey, { local: [mark] }]]); + const delta: DeltaFieldMap = new Map([[rootFieldKey, [mark]]]); applyTestDelta(delta, forest); // Inspect resulting tree: should just have `2`. @@ -762,7 +747,7 @@ export function testForest(config: ForestTestConfiguration): void { count: 1, detach: detachId, }; - const delta: DeltaFieldMap = new Map([[rootFieldKey, { local: [skip, mark] }]]); + const delta: DeltaFieldMap = new Map([[rootFieldKey, [skip, mark]]]); applyTestDelta(delta, forest); // Inspect resulting tree: should just have `1`. @@ -783,11 +768,11 @@ export function testForest(config: ForestTestConfiguration): void { ); const delta: DeltaFieldMap = new Map([ - [rootFieldKey, { local: [{ count: 1, attach: buildId }] }], - ]); - applyTestDelta(delta, forest, undefined, undefined, [ - { id: buildId, trees: [singleJsonCursor(3)] }, + [rootFieldKey, [{ count: 1, attach: buildId }]], ]); + applyTestDelta(delta, forest, { + build: [{ id: buildId, trees: [singleJsonCursor(3)] }], + }); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -812,24 +797,16 @@ export function testForest(config: ForestTestConfiguration): void { count: 1, attach: moveId, }; - const delta: DeltaFieldMap = new Map([ - [ - rootFieldKey, + const delta: DeltaFieldMap = new Map([[rootFieldKey, [moveIn]]]); + applyTestDelta(delta, forest, { + build: [{ id: buildId, trees: [singleJsonCursor({ x: 0 })] }], + global: [ { - global: [ - { - id: buildId, - fields: new Map([[xField, { local: [moveOut] }]]), - }, - ], - local: [moveIn], - relocate: [{ id: buildId, count: 1, destination: detachId }], + id: buildId, + fields: new Map([[xField, [moveOut]]]), }, ], - ]); - applyTestDelta(delta, forest, undefined, undefined, [ - { id: buildId, trees: [singleJsonCursor({ x: 0 })] }, - ]); + }); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -859,11 +836,11 @@ export function testForest(config: ForestTestConfiguration): void { const modify: DeltaMark = { count: 1, fields: new Map([ - [xField, { local: [moveOut] }], - [yField, { local: [{ count: 1 }, moveIn] }], + [xField, [moveOut]], + [yField, [{ count: 1 }, moveIn]], ]), }; - const delta: DeltaFieldMap = new Map([[rootFieldKey, { local: [modify] }]]); + const delta: DeltaFieldMap = new Map([[rootFieldKey, [modify]]]); applyTestDelta(delta, forest); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -886,46 +863,36 @@ export function testForest(config: ForestTestConfiguration): void { ); const delta: DeltaFieldMap = new Map([ - [ - rootFieldKey, + [rootFieldKey, [{ count: 1, attach: buildId }]], + ]); + applyTestDelta(delta, forest, { + build: [ { - global: [ - { - id: buildId, - fields: new Map([ - [ - brand("newField"), - { - local: [{ count: 1, attach: buildId2 }], - }, - ], - ]), - }, + id: buildId, + trees: [ + cursorForJsonableTreeNode({ + type: brand(numberSchema.identifier), + value: 3, + }), + ], + }, + { + id: buildId2, + trees: [ + cursorForJsonableTreeNode({ + type: brand(numberSchema.identifier), + value: 4, + }), ], - local: [{ count: 1, attach: buildId }], }, ], - ]); - applyTestDelta(delta, forest, undefined, undefined, [ - { - id: buildId, - trees: [ - cursorForJsonableTreeNode({ - type: brand(numberSchema.identifier), - value: 3, - }), - ], - }, - { - id: buildId2, - trees: [ - cursorForJsonableTreeNode({ - type: brand(numberSchema.identifier), - value: 4, - }), - ], - }, - ]); + global: [ + { + id: buildId, + fields: new Map([[brand("newField"), [{ count: 1, attach: buildId2 }]]]), + }, + ], + }); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -956,10 +923,10 @@ export function testForest(config: ForestTestConfiguration): void { const mark: DeltaMark = { count: 1, detach: detachId, - fields: new Map([[xField, { local: [{ count: 1, detach: moveId }] }]]), + fields: new Map([[xField, [{ count: 1, detach: moveId }]]]), }; const delta: DeltaFieldMap = new Map([ - [rootFieldKey, { local: [mark, { count: 1, attach: moveId }] }], + [rootFieldKey, [mark, { count: 1, attach: moveId }]], ]); applyTestDelta(delta, forest); @@ -986,36 +953,32 @@ export function testForest(config: ForestTestConfiguration): void { fields: new Map([ [ xField, - { - local: [ - { - count: 1, - detach: moveId, - fields: new Map([ + [ + { + count: 1, + detach: moveId, + fields: new Map([ + [ + fooField, [ - fooField, { - local: [ - { - count: 1, - detach: detachId, - attach: buildId, - }, - ], + count: 1, + detach: detachId, + attach: buildId, }, ], - ]), - }, - ], - }, + ], + ]), + }, + ], ], - [yField, { local: [{ count: 1, attach: moveId }] }], + [yField, [{ count: 1, attach: moveId }]], ]), }; - const delta: DeltaFieldMap = new Map([[rootFieldKey, { local: [mark] }]]); - applyTestDelta(delta, forest, undefined, undefined, [ - { id: buildId, trees: [singleJsonCursor(3)] }, - ]); + const delta: DeltaFieldMap = new Map([[rootFieldKey, [mark]]]); + applyTestDelta(delta, forest, { + build: [{ id: buildId, trees: [singleJsonCursor(3)] }], + }); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -1040,26 +1003,22 @@ export function testForest(config: ForestTestConfiguration): void { const delta: DeltaFieldMap = new Map([ [ rootFieldKey, - { - local: [ - { - count: 1, - fields: new Map([ + [ + { + count: 1, + fields: new Map([ + [ + xField, [ - xField, { - local: [ - { - count: 1, - detach: detachId, - }, - ], + count: 1, + detach: detachId, }, ], - ]), - }, - ], - }, + ], + ]), + }, + ], ], ]); const expected: JsonCompatible[] = [{ y: 1 }]; @@ -1104,11 +1063,11 @@ export function testForest(config: ForestTestConfiguration): void { const modify: DeltaMark = { count: 1, fields: new Map([ - [xField, { local: [moveOut] }], - [yField, { local: [moveIn] }], + [xField, [moveOut]], + [yField, [moveIn]], ]), }; - const delta: DeltaFieldMap = new Map([[rootFieldKey, { local: [modify] }]]); + const delta: DeltaFieldMap = new Map([[rootFieldKey, [modify]]]); applyTestDelta(delta, forest); const expectedCursor = cursorFromInsertable(NodeSchema, { y: 2 }); const expected: JsonableTree[] = [jsonableTreeFromCursor(expectedCursor)]; @@ -1140,17 +1099,15 @@ export function testForest(config: ForestTestConfiguration): void { ); forest.registerAnnouncedVisitor(acquireVisitor); - const delta: DeltaFieldMap = new Map([ - [rootFieldKey, { local: [{ count: 1, attach: buildId }] }], - ]); - applyTestDelta(delta, forest, undefined, undefined, [ - { id: buildId, trees: [singleJsonCursor(3)] }, - ]); + const delta: DeltaFieldMap = new Map([[rootFieldKey, [{ count: 1, attach: buildId }]]]); + applyTestDelta(delta, forest, { + build: [{ id: buildId, trees: [singleJsonCursor(3)] }], + }); forest.deregisterAnnouncedVisitor(acquireVisitor); - applyTestDelta(delta, forest, undefined, undefined, [ - { id: buildId, trees: [singleJsonCursor(4)] }, - ]); + applyTestDelta(delta, forest, { + build: [{ id: buildId, trees: [singleJsonCursor(4)] }], + }); assert.equal(treesCreated, 1); }); diff --git a/packages/dds/tree/src/test/testChange.ts b/packages/dds/tree/src/test/testChange.ts index 360a0afd5332..ef8953cbae32 100644 --- a/packages/dds/tree/src/test/testChange.ts +++ b/packages/dds/tree/src/test/testChange.ts @@ -193,7 +193,7 @@ function toDelta({ change }: TaggedChange): DeltaFieldMap { // We represent the intentions as a list if node offsets in some imaginary field "testIntentions". // This is purely for the sake of testing. brand("testIntentions"), - { local: change.intentions.map((i) => ({ count: i })) }, + change.intentions.map((i) => ({ count: i })), ], ]); } @@ -337,7 +337,7 @@ export function asDelta(intentions: number[]): DeltaRoot { return intentions.length === 0 ? emptyDelta : { - fields: new Map([[rootKey, { local: intentions.map((i) => ({ count: i })) }]]), + fields: new Map([[rootKey, intentions.map((i) => ({ count: i }))]]), }; } diff --git a/packages/dds/tree/src/test/tree/anchorSet.spec.ts b/packages/dds/tree/src/test/tree/anchorSet.spec.ts index cacff34f634d..c287eeaab6d8 100644 --- a/packages/dds/tree/src/test/tree/anchorSet.spec.ts +++ b/packages/dds/tree/src/test/tree/anchorSet.spec.ts @@ -83,9 +83,7 @@ describe("AnchorSet", () => { attach: moveId, }; - const delta = new Map([ - [rootFieldKey, { local: [{ count: 1 }, moveOut, { count: 1 }, moveIn] }], - ]); + const delta = new Map([[rootFieldKey, [{ count: 1 }, moveOut, { count: 1 }, moveIn]]]); announceTestDelta(delta, anchors); checkEquality(anchors.locate(anchor0), makePath([rootFieldKey, 0])); checkEquality(anchors.locate(anchor1), makePath([rootFieldKey, 2])); @@ -97,16 +95,10 @@ describe("AnchorSet", () => { const [anchors, anchor1, anchor2, anchor3] = setup(); const trees = [node, node].map(cursorForJsonableTreeNode); - const fieldChanges: DeltaFieldChanges = { - local: [{ count: 4 }, { count: 2, attach: buildId }], - }; - announceTestDelta( - makeFieldDelta(fieldChanges, makeFieldPath(fieldFoo)), - anchors, - undefined, - undefined, - [{ id: buildId, trees }], - ); + const fieldChanges: DeltaFieldChanges = [{ count: 4 }, { count: 2, attach: buildId }]; + announceTestDelta(makeFieldDelta(fieldChanges, makeFieldPath(fieldFoo)), anchors, { + build: [{ id: buildId, trees }], + }); checkEquality(anchors.locate(anchor1), makePath([fieldFoo, 7], [fieldBar, 4])); checkEquality(anchors.locate(anchor2), makePath([fieldFoo, 3], [fieldBaz, 2])); @@ -233,12 +225,10 @@ describe("AnchorSet", () => { const modify: DeltaMark = { count: 1, - fields: new Map([[fieldBar, { local: [{ count: 3 }, moveIn] }]]), + fields: new Map([[fieldBar, [{ count: 3 }, moveIn]]]), }; - const delta = new Map([ - [fieldFoo, { local: [{ count: 3 }, moveOut, { count: 1 }, modify] }], - ]); + const delta = new Map([[fieldFoo, [{ count: 3 }, moveOut, { count: 1 }, modify]]]); announceTestDelta(delta, anchors); checkEquality(anchors.locate(anchor1), makePath([fieldFoo, 4], [fieldBar, 5])); checkEquality( @@ -260,11 +250,9 @@ describe("AnchorSet", () => { testIdCompressor, ); - announceTestDelta( - makeDelta(detachMark, makePath([fieldFoo, 3])), - anchors, + announceTestDelta(makeDelta(detachMark, makePath([fieldFoo, 3])), anchors, { detachedFieldIndex, - ); + }); checkEquality(anchors.locate(anchor1), makePath([fieldFoo, 4], [fieldBar, 4])); checkRemoved(anchors.locate(anchor2), brand("repair-0")); checkEquality(anchors.locate(anchor3), makePath([fieldFoo, 3])); @@ -275,11 +263,9 @@ describe("AnchorSet", () => { attach: detachId, }; - announceTestDelta( - makeDelta(restoreMark, makePath([fieldFoo, 3])), - anchors, + announceTestDelta(makeDelta(restoreMark, makePath([fieldFoo, 3])), anchors, { detachedFieldIndex, - ); + }); checkEquality(anchors.locate(anchor1), path1); checkEquality(anchors.locate(anchor2), path2); checkEquality(anchors.locate(anchor3), path3); @@ -298,11 +284,9 @@ describe("AnchorSet", () => { testIdCompressor, ); - announceTestDelta( - makeDelta(detachMark, makePath([fieldFoo, 3])), - anchors, + announceTestDelta(makeDelta(detachMark, makePath([fieldFoo, 3])), anchors, { detachedFieldIndex, - ); + }); checkRemoved(anchors.locate(anchor1), brand("repair-2")); checkRemoved(anchors.locate(anchor2), brand("repair-0")); checkRemoved(anchors.locate(anchor3), brand("repair-1")); @@ -313,11 +297,9 @@ describe("AnchorSet", () => { attach: { minor: 44 }, }; - announceTestDelta( - makeDelta(restoreMark, makePath([fieldFoo, 3])), - anchors, + announceTestDelta(makeDelta(restoreMark, makePath([fieldFoo, 3])), anchors, { detachedFieldIndex, - ); + }); checkEquality(anchors.locate(anchor1), makePath([fieldFoo, 3], [fieldBar, 4])); checkRemoved(anchors.locate(anchor2), brand("repair-0")); checkRemoved(anchors.locate(anchor3), brand("repair-1")); @@ -425,7 +407,7 @@ describe("AnchorSet", () => { }; log.expect([]); - announceTestDelta(new Map([[rootFieldKey, { local: [detachMark] }]]), anchors); + announceTestDelta(new Map([[rootFieldKey, [detachMark]]]), anchors); log.expect([ ["root childrenChange", 1], @@ -455,20 +437,7 @@ describe("AnchorSet", () => { ], }, ]; - announceTestDelta( - new Map([ - [ - rootFieldKey, - { - local: [detachMark, insertMark], - }, - ], - ]), - anchors, - undefined, - undefined, - build, - ); + announceTestDelta(new Map([[rootFieldKey, [detachMark, insertMark]]]), anchors, { build }); log.expect([ ["root childrenChange", 2], @@ -477,17 +446,15 @@ describe("AnchorSet", () => { log.clear(); const insertAtFoo5 = makeFieldDelta( - { - local: [{ count: 5 }, insertMark], - }, + [{ count: 5 }, insertMark], makeFieldPath(fieldFoo, [rootFieldKey, 0]), ); - announceTestDelta(insertAtFoo5, anchors, undefined, undefined, build); + announceTestDelta(insertAtFoo5, anchors, { build }); log.expect([["root treeChange", 1]]); log.clear(); - announceTestDelta(new Map([[rootFieldKey, { local: [detachMark] }]]), anchors); + announceTestDelta(new Map([[rootFieldKey, [detachMark]]]), anchors); log.expect([ ["root childrenChange", 1], ["root treeChange", 1], @@ -547,9 +514,7 @@ describe("AnchorSet", () => { }, ]; const insertAtFoo4 = makeFieldDelta( - { - local: [{ count: 4 }, { count: 1, attach: buildId }], - }, + [{ count: 4 }, { count: 1, attach: buildId }], makeFieldPath(fieldFoo, [rootFieldKey, 0]), ); const detachMark: DeltaMark = { @@ -562,23 +527,17 @@ describe("AnchorSet", () => { detach: { minor: 42 }, }; const replaceAtFoo5 = makeFieldDelta( - { - local: [{ count: 5 }, replaceMark], - }, + [{ count: 5 }, replaceMark], makeFieldPath(fieldFoo, [rootFieldKey, 0]), ); const log = new UnorderedTestLogger(); const anchors = new AnchorSet(); const trees = [cursorForJsonableTreeNode(node)]; - const fieldChanges: DeltaFieldChanges = { - local: [{ count: 3 }, { count: 1, attach: buildId }], - }; + const fieldChanges: DeltaFieldChanges = [{ count: 3 }, { count: 1, attach: buildId }]; announceTestDelta( makeFieldDelta(fieldChanges, makeFieldPath(fieldFoo, [rootFieldKey, 0])), anchors, - undefined, - undefined, - [{ id: buildId, trees }], + { build: [{ id: buildId, trees }] }, ); const anchor0 = anchors.track(makePath([rootFieldKey, 0])); const node0 = anchors.locate(anchor0) ?? assert.fail(); @@ -657,13 +616,13 @@ describe("AnchorSet", () => { "subtreeChanging", (n: AnchorNode) => pathVisitor, ); - announceTestDelta(insertAtFoo4, anchors, undefined, undefined, build); + announceTestDelta(insertAtFoo4, anchors, { build }); log.expect([ ["visitSubtreeChange.beforeAttach-src:Temp-0[0, 1]-dst:foo[4]", 1], ["visitSubtreeChange.afterAttach-src:Temp-0[0]-dst:foo[4, 5]", 1], ]); log.clear(); - announceTestDelta(replaceAtFoo5, anchors, undefined, undefined, build); + announceTestDelta(replaceAtFoo5, anchors, { build }); log.expect([ ["visitSubtreeChange.beforeReplace-old:foo[5, 6]-new:Temp-0[0, 1]", 1], ["visitSubtreeChange.afterReplace-old:Temp-1[0, 1]-new:foo[5, 6]", 1], @@ -679,7 +638,7 @@ describe("AnchorSet", () => { ]); log.clear(); unsubscribePathVisitor(); - announceTestDelta(insertAtFoo4, anchors, undefined, undefined, build); + announceTestDelta(insertAtFoo4, anchors, { build }); log.expect([]); }); @@ -783,7 +742,7 @@ function checkRemoved(path: UpPath | undefined, expected: FieldKey = brand("Temp function makeDelta(mark: DeltaMark, path: UpPath): DeltaFieldMap { const fields: DeltaFieldMap = new Map([ - [path.parentField, { local: [{ count: path.parentIndex }, mark] }], + [path.parentField, [{ count: path.parentIndex }, mark]], ]); if (path.parent === undefined) { return fields; diff --git a/packages/dds/tree/src/test/tree/visitDelta.spec.ts b/packages/dds/tree/src/test/tree/visitDelta.spec.ts index 562994fb5c46..5453d1e8b154 100644 --- a/packages/dds/tree/src/test/tree/visitDelta.spec.ts +++ b/packages/dds/tree/src/test/tree/visitDelta.spec.ts @@ -6,9 +6,7 @@ import { strict as assert } from "node:assert"; import { - type DeltaDetachedNodeBuild, type DeltaDetachedNodeChanges, - type DeltaDetachedNodeDestruction, type DeltaDetachedNodeRename, type DeltaFieldChanges, type DeltaMark, @@ -26,6 +24,7 @@ import { rootFromDeltaFieldMap, testIdCompressor, testRevisionTagCodec, + type DeltaParams, } from "../utils.js"; import { deepFreeze } from "@fluidframework/test-runtime-utils/internal"; import { singleJsonCursor } from "../json/index.js"; @@ -65,7 +64,15 @@ const visitorMethods: (keyof DeltaVisitor)[] = [ "exitField", ]; -function testVisit( +/** + * Calls visitDelta on a DeltaRoot and checks that the result is as expected. + * + * @param delta - the root delta to visit + * @param expected - the expected result of the call to visitDelta + * @param detachedFieldIndex - an optional detached field index to get refresher data from + * @param revision - an optional revision for the delta being visited + */ +function testDeltaVisit( delta: DeltaRoot, expected: Readonly, detachedFieldIndex?: DetachedFieldIndex, @@ -89,16 +96,23 @@ function testVisit( assert.deepEqual(result, expected); } +/** + * Creates a DeltaRoot from the provided parameters and calls `testDeltaVisit` on the result. + */ function testTreeVisit( marks: DeltaFieldChanges, expected: Readonly, - detachedFieldIndex?: DetachedFieldIndex, - revision?: RevisionTag, - build?: readonly DeltaDetachedNodeBuild[], - destroy?: readonly DeltaDetachedNodeDestruction[], + params?: DeltaParams, ): void { - const rootDelta = rootFromDeltaFieldMap(new Map([[rootKey, marks]]), build, destroy); - testVisit(rootDelta, expected, detachedFieldIndex, revision); + const { detachedFieldIndex, revision, global, rename, build, destroy } = params ?? {}; + const rootDelta = rootFromDeltaFieldMap( + new Map([[rootKey, marks]]), + global, + rename, + build, + destroy, + ); + testDeltaVisit(rootDelta, expected, detachedFieldIndex, revision); } const rootKey: FieldKey = brand("root"); @@ -112,19 +126,20 @@ const field3: FieldKey = brand("-3"); describe("visitDelta", () => { it("empty delta", () => { - testTreeVisit({}, [ - ["enterField", rootKey], - ["exitField", rootKey], - ["enterField", rootKey], - ["exitField", rootKey], - ]); + testTreeVisit( + [], + [ + ["enterField", rootKey], + ["exitField", rootKey], + ["enterField", rootKey], + ["exitField", rootKey], + ], + ); }); it("insert root", () => { const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); const node = { minor: 42 }; - const rootFieldDelta: DeltaFieldChanges = { - local: [{ count: 1, attach: node }], - }; + const rootFieldDelta: DeltaFieldChanges = [{ count: 1, attach: node }]; const delta: DeltaRoot = { build: [{ id: node, trees: [content] }], fields: new Map([[rootKey, rootFieldDelta]]), @@ -137,34 +152,30 @@ describe("visitDelta", () => { ["attach", field0, 1, 0], ["exitField", rootKey], ]; - testVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.equal(index.entries().next().done, true); }); it("throws on build of existing tree", () => { const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); const node = { minor: 42 }; index.createEntry(node); - const rootFieldDelta: DeltaFieldChanges = { - local: [{ count: 1, attach: node }], - }; + const rootFieldDelta: DeltaFieldChanges = [{ count: 1, attach: node }]; const delta: DeltaRoot = { build: [{ id: node, trees: [content] }], fields: new Map([[rootKey, rootFieldDelta]]), }; - assert.throws(() => testVisit(delta, [], index)); + assert.throws(() => testDeltaVisit(delta, [], index)); assert.deepEqual(Array.from(index.entries()), [{ id: { minor: 42 }, root: 0 }]); }); it("insert child", () => { const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); const buildId = { minor: 42 }; - const rootFieldDelta: DeltaFieldChanges = { - local: [ - { - count: 1, - fields: new Map([[fooKey, { local: [{ count: 1, attach: buildId }] }]]), - }, - ], - }; + const rootFieldDelta: DeltaFieldChanges = [ + { + count: 1, + fields: new Map([[fooKey, [{ count: 1, attach: buildId }]]]), + }, + ]; const expected: VisitScript = [ ["create", [content], field0], ["enterField", rootKey], @@ -185,7 +196,7 @@ describe("visitDelta", () => { build: [{ id: buildId, trees: [content] }], fields: new Map([[rootKey, rootFieldDelta]]), }; - testVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.equal(index.entries().next().done, true); }); it("remove root", () => { @@ -194,9 +205,9 @@ describe("visitDelta", () => { count: 2, detach: { minor: 42 }, }; - const delta = { local: [{ count: 1 }, mark] }; + const marks = [{ count: 1 }, mark]; testTreeVisit( - delta, + marks, [ ["enterField", rootKey], ["detach", { start: 1, end: 2 }, field0], @@ -205,7 +216,9 @@ describe("visitDelta", () => { ["enterField", rootKey], ["exitField", rootKey], ], - index, + { + detachedFieldIndex: index, + }, ); assert.deepEqual(Array.from(index.entries()), [ { id: { minor: 42 }, root: 0 }, @@ -220,9 +233,8 @@ describe("visitDelta", () => { }; const mark: DeltaMark = { count: 1, - fields: new Map([[fooKey, { local: [{ count: 42 }, remove] }]]), + fields: new Map([[fooKey, [{ count: 42 }, remove]]]), }; - const delta = { local: [mark] }; const expected: VisitScript = [ ["enterField", rootKey], ["enterNode", 0], @@ -238,7 +250,7 @@ describe("visitDelta", () => { ["exitNode", 0], ["exitField", rootKey], ]; - testTreeVisit(delta, expected, index); + testTreeVisit([mark], expected, { detachedFieldIndex: index }); assert.deepEqual(Array.from(index.entries()), [{ id: { minor: 42 }, root: 0 }]); }); it("changes under insert", () => { @@ -252,14 +264,15 @@ describe("visitDelta", () => { count: 1, attach: moveId, }; - const delta: DeltaFieldChanges = { + const delta: DeltaRoot = { global: [ { id: { minor: 43 }, - fields: new Map([[fooKey, { local: [{ count: 42 }, moveOut, moveIn] }]]), + fields: new Map([[fooKey, [{ count: 42 }, moveOut, moveIn]]]), }, ], - local: [{ count: 1, attach: { minor: 43 } }], + build: [{ id: { minor: 43 }, trees: [content] }], + fields: new Map([[rootKey, [{ count: 1, attach: { minor: 43 } }]]]), }; const expected: VisitScript = [ ["create", [content], field0], @@ -281,9 +294,7 @@ describe("visitDelta", () => { ["exitNode", 0], ["exitField", rootKey], ]; - testTreeVisit(delta, expected, index, undefined, [ - { id: { minor: 43 }, trees: [content] }, - ]); + testDeltaVisit(delta, expected, index); assert.equal(index.entries().next().done, true); }); it("move node to the right", () => { @@ -298,7 +309,7 @@ describe("visitDelta", () => { count: 1, attach: moveId, }; - const delta = { local: [{ count: 1 }, moveOut, { count: 1 }, moveIn] }; + const marks = [{ count: 1 }, moveOut, { count: 1 }, moveIn]; const expected: VisitScript = [ ["enterField", rootKey], ["detach", { start: 1, end: 2 }, field0], @@ -307,7 +318,7 @@ describe("visitDelta", () => { ["attach", field0, 1, 2], ["exitField", rootKey], ]; - testTreeVisit(delta, expected, index); + testTreeVisit(marks, expected, { detachedFieldIndex: index }); assert.equal(index.entries().next().done, true); }); it("move children to the left", () => { @@ -323,9 +334,9 @@ describe("visitDelta", () => { }; const modify: DeltaMark = { count: 1, - fields: new Map([[fooKey, { local: [{ count: 2 }, moveIn, { count: 3 }, moveOut] }]]), + fields: new Map([[fooKey, [{ count: 2 }, moveIn, { count: 3 }, moveOut]]]), }; - const delta = { local: [modify] }; + const marks = [modify]; const expected: VisitScript = [ ["enterField", rootKey], ["enterNode", 0], @@ -344,7 +355,7 @@ describe("visitDelta", () => { ["exitNode", 0], ["exitField", rootKey], ]; - testTreeVisit(delta, expected, index); + testTreeVisit(marks, expected, { detachedFieldIndex: index }); assert.equal(index.entries().next().done, true); }); it("move cousins", () => { @@ -361,11 +372,11 @@ describe("visitDelta", () => { const modify: DeltaMark = { count: 1, fields: new Map([ - [fooKey, { local: [moveIn] }], - [barKey, { local: [moveOut] }], + [fooKey, [moveIn]], + [barKey, [moveOut]], ]), }; - const delta = { local: [modify] }; + const marks = [modify]; const expected: VisitScript = [ ["enterField", rootKey], ["enterNode", 0], @@ -386,7 +397,7 @@ describe("visitDelta", () => { ["exitNode", 0], ["exitField", rootKey], ]; - testTreeVisit(delta, expected, index); + testTreeVisit(marks, expected, { detachedFieldIndex: index }); assert.equal(index.entries().next().done, true); }); it("changes under remove", () => { @@ -403,9 +414,9 @@ describe("visitDelta", () => { const remove: DeltaMark = { detach: { minor: 42 }, count: 1, - fields: new Map([[fooKey, { local: [moveOut, moveIn] }]]), + fields: new Map([[fooKey, [moveOut, moveIn]]]), }; - const delta = { local: [remove] }; + const marks = [remove]; const expected: VisitScript = [ ["enterField", rootKey], ["enterNode", 0], @@ -425,7 +436,7 @@ describe("visitDelta", () => { ["exitNode", 0], ["exitField", field1], ]; - testTreeVisit(delta, expected, index); + testTreeVisit(marks, expected, { detachedFieldIndex: index }); assert.deepEqual(Array.from(index.entries()), [{ id: { minor: 42 }, root: 1 }]); }); it("changes under destroy", () => { @@ -443,10 +454,11 @@ describe("visitDelta", () => { }; const nested: DeltaDetachedNodeChanges = { id: node1, - fields: new Map([[fooKey, { local: [moveOut, moveIn] }]]), + fields: new Map([[fooKey, [moveOut, moveIn]]]), }; - const delta: DeltaFieldChanges = { + const delta: DeltaRoot = { global: [nested], + destroy: [{ id: node1, count: 1 }], }; const expected: VisitScript = [ ["enterField", rootKey], @@ -469,7 +481,7 @@ describe("visitDelta", () => { ["exitField", field0], ["destroy", field0, 1], ]; - testTreeVisit(delta, expected, index, undefined, undefined, [{ id: node1, count: 1 }]); + testDeltaVisit(delta, expected, index); assert.equal(index.entries().next().done, true); }); it("destroy (root level)", () => { @@ -483,15 +495,17 @@ describe("visitDelta", () => { ["destroy", field0, 1], ["destroy", field1, 1], ]; - testVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.equal(index.entries().next().done, true); }); it("build-rename-destroy (field level)", () => { const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); const buildId = { minor: 42 }; const detachId = { minor: 43 }; - const delta: DeltaFieldChanges = { + const delta: DeltaRoot = { rename: [{ oldId: buildId, newId: detachId, count: 1 }], + build: [{ id: buildId, trees: [content] }], + destroy: [{ id: detachId, count: 1 }], }; const expected: VisitScript = [ ["create", [content], field0], @@ -504,14 +518,7 @@ describe("visitDelta", () => { ["exitField", rootKey], ["destroy", field1, 1], ]; - testTreeVisit( - delta, - expected, - index, - undefined, - [{ id: buildId, trees: [content] }], - [{ id: detachId, count: 1 }], - ); + testDeltaVisit(delta, expected, index); assert.equal(index.entries().next().done, true); }); it("changes under move-out", () => { @@ -533,9 +540,9 @@ describe("visitDelta", () => { const moveOut1: DeltaMark = { count: 1, detach: moveId1, - fields: new Map([[fooKey, { local: [moveOut2, moveIn2] }]]), + fields: new Map([[fooKey, [moveOut2, moveIn2]]]), }; - const delta = { local: [moveOut1, moveIn1] }; + const marks = [moveOut1, moveIn1]; const expected: VisitScript = [ ["enterField", rootKey], ["enterNode", 0], @@ -554,7 +561,7 @@ describe("visitDelta", () => { ["exitNode", 0], ["exitField", rootKey], ]; - testTreeVisit(delta, expected, index); + testTreeVisit(marks, expected, { detachedFieldIndex: index }); assert.equal(index.entries().next().done, true); }); @@ -577,14 +584,14 @@ describe("visitDelta", () => { const moveOut2: DeltaMark = { count: 1, detach: { minor: 3 }, - fields: new Map([[fooKey, { local: [attach] }]]), + fields: new Map([[fooKey, [attach]]]), }; - const rootChanges: DeltaFieldChanges = { local: [moveOut1, moveOut2, moveIn] }; + const fieldChanges: DeltaFieldChanges = [moveOut1, moveOut2, moveIn]; const delta: DeltaRoot = { build: [{ id: buildId, trees: [content] }], - fields: new Map([[rootKey, rootChanges]]), + fields: new Map([[rootKey, fieldChanges]]), }; const expected: VisitScript = [ @@ -608,7 +615,7 @@ describe("visitDelta", () => { ["exitField", rootKey], ]; - testVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.equal(index.entries().next().done, true); }); @@ -621,10 +628,10 @@ describe("visitDelta", () => { attach: buildId, }; - const rootChanges: DeltaFieldChanges = { local: [replace] }; + const fieldChanges: DeltaFieldChanges = [replace]; const delta: DeltaRoot = { build: [{ id: buildId, trees: [content, content] }], - fields: new Map([[rootKey, rootChanges]]), + fields: new Map([[rootKey, fieldChanges]]), }; const expected: VisitScript = [ @@ -639,7 +646,7 @@ describe("visitDelta", () => { ]; const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); - testVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); }); it("changes under replaced node", () => { @@ -662,9 +669,9 @@ describe("visitDelta", () => { count: 1, detach: { minor: 42 }, attach: moveId1, - fields: new Map([[fooKey, { local: [moveOut2, moveIn2] }]]), + fields: new Map([[fooKey, [moveOut2, moveIn2]]]), }; - const delta = { local: [replace, moveOut1] }; + const marks = [replace, moveOut1]; const expected: VisitScript = [ ["enterField", rootKey], ["enterNode", 0], @@ -685,7 +692,7 @@ describe("visitDelta", () => { ["exitNode", 0], ["exitField", field2], ]; - testTreeVisit(delta, expected, index); + testTreeVisit(marks, expected, { detachedFieldIndex: index }); assert.deepEqual(Array.from(index.entries()), [{ id: { minor: 42 }, root: 2 }]); }); @@ -704,14 +711,14 @@ describe("visitDelta", () => { const moveOut1: DeltaMark = { count: 1, detach: moveId1, - fields: new Map([[fooKey, { local: [moveOut2, moveIn2] }]]), + fields: new Map([[fooKey, [moveOut2, moveIn2]]]), }; const replace: DeltaMark = { count: 1, detach: { minor: 42 }, attach: moveId1, }; - const delta = { local: [replace, moveOut1] }; + const marks = [replace, moveOut1]; const expected: VisitScript = [ ["enterField", rootKey], ["enterNode", 1], @@ -730,12 +737,13 @@ describe("visitDelta", () => { ["exitNode", 0], ["exitField", rootKey], ]; - testTreeVisit(delta, expected, index); + testTreeVisit(marks, expected, { detachedFieldIndex: index }); assert.deepEqual(Array.from(index.entries()), [{ id: { minor: 42 }, root: 2 }]); }); it("transient insert", () => { const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); - const delta: DeltaFieldChanges = { + const delta: DeltaRoot = { + build: [{ id: { minor: 42 }, trees: [content] }], rename: [{ oldId: { minor: 42 }, count: 1, newId: { minor: 43 } }], }; const expected: VisitScript = [ @@ -748,9 +756,7 @@ describe("visitDelta", () => { ["enterField", rootKey], ["exitField", rootKey], ]; - testTreeVisit(delta, expected, index, undefined, [ - { id: { minor: 42 }, trees: [content] }, - ]); + testDeltaVisit(delta, expected, index); assert.deepEqual(Array.from(index.entries()), [{ id: { minor: 43 }, root: 1 }]); }); it("changes under transient", () => { @@ -766,8 +772,9 @@ describe("visitDelta", () => { }; const buildId = { minor: 42 }; const detachId = { minor: 43 }; - const delta: DeltaFieldChanges = { - global: [{ id: buildId, fields: new Map([[barKey, { local: [moveOut, moveIn] }]]) }], + const delta: DeltaRoot = { + build: [{ id: buildId, trees: [content] }], + global: [{ id: buildId, fields: new Map([[barKey, [moveOut, moveIn]]]) }], rename: [{ oldId: buildId, count: 1, newId: detachId }], }; const expected: VisitScript = [ @@ -794,7 +801,7 @@ describe("visitDelta", () => { ["exitNode", 0], ["exitField", field2], ]; - testTreeVisit(delta, expected, index, undefined, [{ id: buildId, trees: [content] }]); + testDeltaVisit(delta, expected, index); assert.deepEqual(Array.from(index.entries()), [{ id: detachId, root: 2 }]); }); it("restore", () => { @@ -805,7 +812,7 @@ describe("visitDelta", () => { count: 1, attach: node1, }; - const delta = { local: [restore] }; + const marks = [restore]; const expected: VisitScript = [ ["enterField", rootKey], ["exitField", rootKey], @@ -813,7 +820,7 @@ describe("visitDelta", () => { ["attach", field0, 1, 0], ["exitField", rootKey], ]; - testTreeVisit(delta, expected, index); + testTreeVisit(marks, expected, { detachedFieldIndex: index }); assert.equal(index.entries().next().done, true); }); it("move removed node", () => { @@ -830,10 +837,7 @@ describe("visitDelta", () => { count: 1, attach: moveId, }; - const delta = { - rename: [rename], - local: [moveIn], - }; + const marks = [moveIn]; const expected: VisitScript = [ ["enterField", rootKey], ["exitField", rootKey], @@ -844,7 +848,7 @@ describe("visitDelta", () => { ["attach", field1, 1, 0], ["exitField", rootKey], ]; - testTreeVisit(delta, expected, index); + testTreeVisit(marks, expected, { detachedFieldIndex: index, rename: [rename] }); assert.equal(index.entries().next().done, true); }); it("changes under removed node", () => { @@ -862,7 +866,7 @@ describe("visitDelta", () => { }; const modify: DeltaDetachedNodeChanges = { id: { minor: 1 }, - fields: new Map([[fooKey, { local: [moveOut, moveIn] }]]), + fields: new Map([[fooKey, [moveOut, moveIn]]]), }; const delta = { global: [modify] }; const expected: VisitScript = [ @@ -885,7 +889,7 @@ describe("visitDelta", () => { ["exitNode", 0], ["exitField", field0], ]; - testTreeVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.deepEqual(Array.from(index.entries()), [{ id: { minor: 1 }, root: 0 }]); }); it("changes under transient move-in", () => { @@ -899,12 +903,10 @@ describe("visitDelta", () => { fields: new Map([ [ fooKey, - { - local: [ - { count: 1, detach: moveId2 }, - { count: 1, attach: moveId2 }, - ], - }, + [ + { count: 1, detach: moveId2 }, + { count: 1, attach: moveId2 }, + ], ], ]), }; @@ -913,7 +915,7 @@ describe("visitDelta", () => { oldId: moveId1, newId: detachId, }; - const delta = { local: [moveOut], rename: [moveIn] }; + const delta: DeltaRoot = { fields: new Map([[rootKey, [moveOut]]]), rename: [moveIn] }; const expected: VisitScript = [ ["enterField", rootKey], ["enterNode", 0], @@ -936,7 +938,7 @@ describe("visitDelta", () => { ["exitNode", 0], ["exitField", field2], ]; - testTreeVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.deepEqual(Array.from(index.entries()), [{ id: detachId, root: 2 }]); }); it("transient restore", () => { @@ -958,7 +960,7 @@ describe("visitDelta", () => { ["enterField", rootKey], ["exitField", rootKey], ]; - testTreeVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.deepEqual(Array.from(index.entries()), [{ id: { minor: 42 }, root: 1 }]); }); it("update detached node", () => { @@ -978,6 +980,7 @@ describe("visitDelta", () => { newId: node1, }; const delta = { + build: [{ id: buildId, trees: [content] }], rename: [renameOldNode, renameNewNode], }; const expected: VisitScript = [ @@ -993,7 +996,7 @@ describe("visitDelta", () => { ["enterField", rootKey], ["exitField", rootKey], ]; - testTreeVisit(delta, expected, index, undefined, [{ id: buildId, trees: [content] }]); + testDeltaVisit(delta, expected, index); assert.deepEqual(Array.from(index.entries()), [ { id: detachId, root: 2 }, { id: node1, root: 3 }, @@ -1004,9 +1007,7 @@ describe("visitDelta", () => { it("for restores at the root", () => { const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); const node = { minor: 42 }; - const rootFieldDelta: DeltaFieldChanges = { - local: [{ count: 1, attach: node }], - }; + const rootFieldDelta: DeltaFieldChanges = [{ count: 1, attach: node }]; const delta: DeltaRoot = { refreshers: [{ id: node, trees: [content] }], fields: new Map([[rootKey, rootFieldDelta]]), @@ -1019,21 +1020,19 @@ describe("visitDelta", () => { ["attach", field0, 1, 0], ["exitField", rootKey], ]; - testVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.equal(index.entries().next().done, true); }); it("for restores under a child", () => { const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); const buildId = { minor: 42 }; - const rootFieldDelta: DeltaFieldChanges = { - local: [ - { - count: 1, - fields: new Map([[fooKey, { local: [{ count: 1, attach: buildId }] }]]), - }, - ], - }; + const rootFieldDelta: DeltaFieldChanges = [ + { + count: 1, + fields: new Map([[fooKey, [{ count: 1, attach: buildId }]]]), + }, + ]; const expected: VisitScript = [ ["enterField", rootKey], ["enterNode", 0], @@ -1054,16 +1053,14 @@ describe("visitDelta", () => { refreshers: [{ id: buildId, trees: [content] }], fields: new Map([[rootKey, rootFieldDelta]]), }; - testVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.equal(index.entries().next().done, true); }); it("for partial restores", () => { const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); const node = { minor: 42 }; - const rootFieldDelta: DeltaFieldChanges = { - local: [{ count: 1, attach: { minor: 43 } }], - }; + const rootFieldDelta: DeltaFieldChanges = [{ count: 1, attach: { minor: 43 } }]; const delta: DeltaRoot = { refreshers: [{ id: node, trees: [content, content] }], fields: new Map([[rootKey, rootFieldDelta]]), @@ -1076,7 +1073,7 @@ describe("visitDelta", () => { ["attach", field0, 1, 0], ["exitField", rootKey], ]; - testVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.equal(index.entries().next().done, true); }); @@ -1084,14 +1081,6 @@ describe("visitDelta", () => { const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); const refresherId = { minor: 42 }; const buildId = { minor: 43 }; - const rootFieldDelta: DeltaFieldChanges = { - global: [ - { - id: refresherId, - fields: new Map([[fooKey, { local: [{ count: 1, attach: buildId }] }]]), - }, - ], - }; const expected: VisitScript = [ ["create", [content], field0], ["enterField", rootKey], @@ -1114,11 +1103,18 @@ describe("visitDelta", () => { ["exitField", field1], ]; const delta: DeltaRoot = { + global: [ + { + id: refresherId, + fields: new Map([[fooKey, [{ count: 1, attach: buildId }]]]), + }, + ], refreshers: [{ id: refresherId, trees: [content] }], build: [{ id: buildId, trees: [content] }], - fields: new Map([[rootKey, rootFieldDelta]]), + // TODO the global was in this so it might've changed the expected value + // fields: new Map([[rootKey, rootFieldDelta]]), }; - testVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); }); }); @@ -1146,7 +1142,7 @@ describe("visitDelta", () => { ["exitField", rootKey], ]; const revision = mintRevisionTag(); - testTreeVisit(delta, expected, index, revision); + testDeltaVisit(delta, expected, index, revision); const iteratorResult = index.entries().next(); assert.equal(iteratorResult.done, false); assert.deepEqual(iteratorResult.value.id, moveId); @@ -1162,7 +1158,7 @@ describe("visitDelta", () => { }; const expected: VisitScript = [["create", [content], field0]]; const revision = mintRevisionTag(); - testVisit(delta, expected, index, revision); + testDeltaVisit(delta, expected, index, revision); assert.deepEqual(Array.from(index.entries()), [ { id: node, root: 0, latestRelevantRevision: revision }, ]); @@ -1183,9 +1179,9 @@ describe("visitDelta", () => { }; const modify: DeltaDetachedNodeChanges = { id: node1, - fields: new Map([[fooKey, { local: [moveOut, moveIn] }]]), + fields: new Map([[fooKey, [moveOut, moveIn]]]), }; - const delta = { global: [modify] }; + const global = [modify]; const expected: VisitScript = [ ["enterField", rootKey], ["exitField", rootKey], @@ -1207,7 +1203,7 @@ describe("visitDelta", () => { ["exitField", field0], ]; const revision = mintRevisionTag(); - testTreeVisit(delta, expected, index, revision); + testTreeVisit([], expected, { detachedFieldIndex: index, revision, global }); assert.deepEqual(Array.from(index.entries()), [ { id: node1, root: 0, latestRelevantRevision: revision }, ]); @@ -1221,7 +1217,7 @@ describe("visitDelta", () => { detach: { minor: 2 }, }; - const rootChanges: DeltaFieldChanges = { local: [replace] }; + const rootChanges: DeltaFieldChanges = [replace]; const delta: DeltaRoot = { build: [{ id: buildId, trees: [content, content] }], fields: new Map([[rootKey, rootChanges]]), @@ -1240,7 +1236,7 @@ describe("visitDelta", () => { const revision = mintRevisionTag(); const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); - testVisit(delta, expected, index, revision); + testDeltaVisit(delta, expected, index, revision); const iteratorResult = index.entries().next(); assert.equal(iteratorResult.done, false); assert.deepEqual(iteratorResult.value.id, buildId); @@ -1253,9 +1249,7 @@ describe("visitDelta", () => { const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); const node = { minor: 42 }; const node2 = { minor: 43 }; - const rootFieldDelta: DeltaFieldChanges = { - local: [{ count: 1, attach: node2 }], - }; + const rootFieldDelta: DeltaFieldChanges = [{ count: 1, attach: node2 }]; const delta: DeltaRoot = { refreshers: [ { id: node, trees: [content] }, @@ -1271,7 +1265,7 @@ describe("visitDelta", () => { ["attach", field0, 1, 0], ["exitField", rootKey], ]; - testVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.equal(index.entries().next().done, true); }); @@ -1279,9 +1273,7 @@ describe("visitDelta", () => { const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); const node = { minor: 42 }; index.createEntry(node, undefined, 1); - const rootFieldDelta: DeltaFieldChanges = { - local: [{ count: 1, attach: node }], - }; + const rootFieldDelta: DeltaFieldChanges = [{ count: 1, attach: node }]; const delta: DeltaRoot = { refreshers: [{ id: node, trees: [content] }], fields: new Map([[rootKey, rootFieldDelta]]), @@ -1293,16 +1285,14 @@ describe("visitDelta", () => { ["attach", field0, 1, 0], ["exitField", rootKey], ]; - testVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.equal(index.entries().next().done, true); }); it("when the refreshed tree is included in the builds", () => { const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); const node = { minor: 42 }; - const rootFieldDelta: DeltaFieldChanges = { - local: [{ count: 1, attach: node }], - }; + const rootFieldDelta: DeltaFieldChanges = [{ count: 1, attach: node }]; const delta: DeltaRoot = { build: [{ id: node, trees: [content] }], refreshers: [{ id: node, trees: [content] }], @@ -1316,7 +1306,7 @@ describe("visitDelta", () => { ["attach", field0, 1, 0], ["exitField", rootKey], ]; - testVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.equal(index.entries().next().done, true); }); }); @@ -1354,7 +1344,7 @@ describe("visitDelta", () => { ["enterField", rootKey], ["exitField", rootKey], ]; - testTreeVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.deepEqual(Array.from(index.entries()), [{ id: end, root: cycle ? 0 : 1 }]); }); }); @@ -1392,7 +1382,7 @@ describe("visitDelta", () => { ["enterField", rootKey], ["exitField", rootKey], ]; - testTreeVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.deepEqual(Array.from(index.entries()), [{ id: end, root: 2 }]); }); } @@ -1444,7 +1434,7 @@ describe("visitDelta", () => { ]; const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); index.createEntry(pointA); - testTreeVisit(delta, expected, index); + testDeltaVisit(delta, expected, index); assert.deepEqual(Array.from(index.entries()), [{ id: end, root: 3 }]); }); } diff --git a/packages/dds/tree/src/test/utils.ts b/packages/dds/tree/src/test/utils.ts index e69d61e45f50..e40fc8a1803a 100644 --- a/packages/dds/tree/src/test/utils.ts +++ b/packages/dds/tree/src/test/utils.ts @@ -108,6 +108,7 @@ import { MockNodeKeyManager, cursorForMapTreeField, type IDefaultEditBuilder, + type FieldChangeDelta, } from "../feature-libraries/index.js"; // eslint-disable-next-line import/no-internal-modules import { makeSchemaCodec } from "../feature-libraries/schema-index/codec.js"; @@ -481,7 +482,7 @@ export function isDeltaVisible(fieldChanges: DeltaFieldChanges): boolean { /** * Assert two MarkList are equal, handling cursors. */ -export function assertFieldChangesEqual(a: DeltaFieldChanges, b: DeltaFieldChanges): void { +export function assertFieldChangesEqual(a: FieldChangeDelta, b: FieldChangeDelta): void { assert.deepStrictEqual(a, b); } @@ -1006,15 +1007,22 @@ export function defaultRevInfosFromChanges( return revInfos; } +export interface DeltaParams { + detachedFieldIndex?: DetachedFieldIndex; + revision?: RevisionTag; + global?: readonly DeltaDetachedNodeChanges[]; + rename?: readonly DeltaDetachedNodeRename[]; + build?: readonly DeltaDetachedNodeBuild[]; + destroy?: readonly DeltaDetachedNodeDestruction[]; +} + export function applyTestDelta( delta: DeltaFieldMap, deltaProcessor: { acquireVisitor: () => DeltaVisitor }, - detachedFieldIndex?: DetachedFieldIndex, - revision?: RevisionTag, - build?: readonly DeltaDetachedNodeBuild[], - destroy?: readonly DeltaDetachedNodeDestruction[], + params?: DeltaParams, ): void { - const rootDelta = rootFromDeltaFieldMap(delta, build, destroy); + const { detachedFieldIndex, revision, global, rename, build, destroy } = params ?? {}; + const rootDelta = rootFromDeltaFieldMap(delta, global, rename, build, destroy); applyDelta( rootDelta, revision, @@ -1027,12 +1035,10 @@ export function applyTestDelta( export function announceTestDelta( delta: DeltaFieldMap, deltaProcessor: { acquireVisitor: () => DeltaVisitor & AnnouncedVisitor }, - detachedFieldIndex?: DetachedFieldIndex, - revision?: RevisionTag, - build?: readonly DeltaDetachedNodeBuild[], - destroy?: readonly DeltaDetachedNodeDestruction[], + params?: DeltaParams, ): void { - const rootDelta = rootFromDeltaFieldMap(delta, build, destroy); + const { detachedFieldIndex, revision, global, rename, build, destroy } = params ?? {}; + const rootDelta = rootFromDeltaFieldMap(delta, global, rename, build, destroy); announceDelta( rootDelta, revision, From 588bbe08902ee16871612b39e24d8902db97202e Mon Sep 17 00:00:00 2001 From: Jenn Date: Wed, 15 Jan 2025 19:45:17 +0000 Subject: [PATCH 09/16] fix most tests --- packages/dds/tree/src/core/index.ts | 1 - packages/dds/tree/src/core/tree/deltaUtil.ts | 7 +-- packages/dds/tree/src/core/tree/index.ts | 1 - packages/dds/tree/src/core/tree/visitDelta.ts | 53 +++++++++++------- .../tree/src/feature-libraries/deltaUtils.ts | 13 +++++ .../modular-schema/modularChangeFamily.ts | 3 +- .../sequence-field/sequenceFieldToDelta.ts | 2 +- .../modular-schema/genericFieldKind.spec.ts | 19 ++++--- .../optionalChangeRebaser.test.ts | 10 ++-- .../feature-libraries/sequence-field/utils.ts | 16 ++---- packages/dds/tree/src/test/testChange.spec.ts | 9 +-- .../dds/tree/src/test/tree/visitDelta.spec.ts | 55 +------------------ packages/dds/tree/src/test/utils.ts | 2 +- 13 files changed, 74 insertions(+), 117 deletions(-) diff --git a/packages/dds/tree/src/core/index.ts b/packages/dds/tree/src/core/index.ts index 496e19aed616..e0fe096710b2 100644 --- a/packages/dds/tree/src/core/index.ts +++ b/packages/dds/tree/src/core/index.ts @@ -68,7 +68,6 @@ export { type PathRootPrefix, deltaForRootInitialization, emptyFieldChanges, - isEmptyFieldChanges, makeDetachedNodeId, offsetDetachId, emptyDelta, diff --git a/packages/dds/tree/src/core/tree/deltaUtil.ts b/packages/dds/tree/src/core/tree/deltaUtil.ts index 1a5c99aee98b..2ef13f83f7b4 100644 --- a/packages/dds/tree/src/core/tree/deltaUtil.ts +++ b/packages/dds/tree/src/core/tree/deltaUtil.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. */ +import type { FieldChangeDelta } from "../../feature-libraries/index.js"; import type { Mutable } from "../../util/index.js"; import type { FieldKey } from "../schema-stored/index.js"; @@ -12,7 +13,7 @@ import { rootFieldKey } from "./types.js"; export const emptyDelta: Root = {}; -export const emptyFieldChanges: FieldChanges = []; +export const emptyFieldChanges: FieldChangeDelta = {}; export function isAttachMark(mark: Mark): boolean { return mark.attach !== undefined && mark.detach === undefined; @@ -26,10 +27,6 @@ export function isReplaceMark(mark: Mark): boolean { return mark.detach !== undefined && mark.attach !== undefined; } -export function isEmptyFieldChanges(fieldChanges: FieldChanges): boolean { - return fieldChanges.length === 0; -} - export function deltaForRootInitialization(content: readonly ITreeCursorSynchronous[]): Root { if (content.length === 0) { return emptyDelta; diff --git a/packages/dds/tree/src/core/tree/index.ts b/packages/dds/tree/src/core/tree/index.ts index edab69d49277..08b1e155995c 100644 --- a/packages/dds/tree/src/core/tree/index.ts +++ b/packages/dds/tree/src/core/tree/index.ts @@ -105,7 +105,6 @@ export { SparseNode, getDescendant } from "./sparseTree.js"; export { deltaForRootInitialization, emptyFieldChanges, - isEmptyFieldChanges, makeDetachedNodeId, offsetDetachId, emptyDelta, diff --git a/packages/dds/tree/src/core/tree/visitDelta.ts b/packages/dds/tree/src/core/tree/visitDelta.ts index d259431d55ba..50e8d7ac2749 100644 --- a/packages/dds/tree/src/core/tree/visitDelta.ts +++ b/packages/dds/tree/src/core/tree/visitDelta.ts @@ -105,6 +105,8 @@ export function visitDelta( rootDestructions, }; processBuilds(delta.build, detachConfig, visitor); + processGlobal(delta.global, detachConfig, visitor); + processRename(delta.rename, detachConfig); visitFieldMarks(delta.fields, visitor, detachConfig); fixedPointVisitOfRoots(visitor, detachPassRoots, detachConfig); transferRoots( @@ -423,27 +425,7 @@ function detachPass( fieldChanges: Delta.FieldChanges, visitor: DeltaVisitor, config: PassConfig, - global?: readonly Delta.DetachedNodeChanges[], - rename?: readonly Delta.DetachedNodeRename[], ): void { - if (global !== undefined) { - for (const { id, fields } of global) { - let root = config.detachedFieldIndex.tryGetEntry(id); - if (root === undefined) { - const tree = tryGetFromNestedMap(config.refreshers, id.major, id.minor); - assert(tree !== undefined, 0x928 /* refresher data not found */); - buildTrees(id, [tree], config.detachedFieldIndex, config.latestRevision, visitor); - root = config.detachedFieldIndex.getEntry(id); - } - // the revision is updated for any refresher data included in the delta that is used - config.detachedFieldIndex.updateLatestRevision(id, config.latestRevision); - config.detachPassRoots.set(root, fields); - config.attachPassRoots.set(root, fields); - } - } - if (rename !== undefined) { - config.rootTransfers.push(...rename); - } let index = 0; for (const mark of fieldChanges) { if (mark.fields !== undefined) { @@ -499,6 +481,37 @@ function processBuilds( } } +function processGlobal( + global: readonly Delta.DetachedNodeChanges[] | undefined, + config: PassConfig, + visitor: DeltaVisitor, +): void { + if (global !== undefined) { + for (const { id, fields } of global) { + let root = config.detachedFieldIndex.tryGetEntry(id); + if (root === undefined) { + const tree = tryGetFromNestedMap(config.refreshers, id.major, id.minor); + assert(tree !== undefined, 0x928 /* refresher data not found */); + buildTrees(id, [tree], config.detachedFieldIndex, config.latestRevision, visitor); + root = config.detachedFieldIndex.getEntry(id); + } + // the revision is updated for any refresher data included in the delta that is used + config.detachedFieldIndex.updateLatestRevision(id, config.latestRevision); + config.detachPassRoots.set(root, fields); + config.attachPassRoots.set(root, fields); + } + } +} + +function processRename( + rename: readonly Delta.DetachedNodeRename[] | undefined, + config: PassConfig, +): void { + if (rename !== undefined) { + config.rootTransfers.push(...rename); + } +} + function collectDestroys( destroys: readonly Delta.DetachedNodeDestruction[] | undefined, config: PassConfig, diff --git a/packages/dds/tree/src/feature-libraries/deltaUtils.ts b/packages/dds/tree/src/feature-libraries/deltaUtils.ts index cd25f76070d8..090d45c4d372 100644 --- a/packages/dds/tree/src/feature-libraries/deltaUtils.ts +++ b/packages/dds/tree/src/feature-libraries/deltaUtils.ts @@ -39,5 +39,18 @@ export function mapRootChanges( trees: trees.map(func), })); } + if (root.global !== undefined) { + out.global = root.global.map(({ id, fields }) => ({ + id, + fields, + })); + } + if (root.rename !== undefined) { + out.rename = root.rename.map(({ count, oldId, newId }) => ({ + count, + oldId, + newId, + })); + } return out; } diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts index 2d833c99e12c..2aad41a03046 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts @@ -30,7 +30,6 @@ import { type RevisionTag, type TaggedChange, type UpPath, - isEmptyFieldChanges, makeDetachedNodeId, mapCursorField, replaceAtomRevisions, @@ -2101,7 +2100,7 @@ function intoDeltaImpl( } }, ); - if (fieldChanges !== undefined && !isEmptyFieldChanges(fieldChanges)) { + if (fieldChanges !== undefined && fieldChanges.length > 0) { delta.set(field, fieldChanges); } if (fieldGlobal !== undefined) { diff --git a/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts b/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts index a33167225957..6ce6753edce5 100644 --- a/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts +++ b/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts @@ -40,7 +40,7 @@ export function sequenceFieldToDelta( const changes = mark.changes; if (changes !== undefined) { const nestedDelta = deltaFromChild(changes); - if (nestedDelta !== undefined) { + if (nestedDelta !== undefined && nestedDelta.size > 0) { if (inputCellId === undefined) { deltaMark.fields = nestedDelta; } else { diff --git a/packages/dds/tree/src/test/feature-libraries/modular-schema/genericFieldKind.spec.ts b/packages/dds/tree/src/test/feature-libraries/modular-schema/genericFieldKind.spec.ts index 0d191f4e09e9..1f22faedf86b 100644 --- a/packages/dds/tree/src/test/feature-libraries/modular-schema/genericFieldKind.spec.ts +++ b/packages/dds/tree/src/test/feature-libraries/modular-schema/genericFieldKind.spec.ts @@ -6,8 +6,11 @@ import { strict as assert } from "node:assert"; import type { SessionId } from "@fluidframework/id-compressor"; -import type { GenericChangeset, CrossFieldManager } from "../../../feature-libraries/index.js"; -import type { DeltaFieldChanges } from "../../../core/index.js"; +import type { + GenericChangeset, + CrossFieldManager, + FieldChangeDelta, +} from "../../../feature-libraries/index.js"; import { fakeIdAllocator, brand, idAllocatorFromMaxId } from "../../../util/index.js"; import { type EncodingTestData, @@ -185,11 +188,13 @@ describe("GenericField", () => { [2, nodeChange2], ]); - const expected: DeltaFieldChanges = [ - { count: 1, fields: TestNodeId.deltaFromChild(nodeChange1) }, - { count: 1 }, - { count: 1, fields: TestNodeId.deltaFromChild(nodeChange2) }, - ]; + const expected: FieldChangeDelta = { + local: [ + { count: 1, fields: TestNodeId.deltaFromChild(nodeChange1) }, + { count: 1 }, + { count: 1, fields: TestNodeId.deltaFromChild(nodeChange2) }, + ], + }; const actual = genericChangeHandler.intoDelta(input, TestNodeId.deltaFromChild); assert.deepEqual(actual, expected); diff --git a/packages/dds/tree/src/test/feature-libraries/optional-field/optionalChangeRebaser.test.ts b/packages/dds/tree/src/test/feature-libraries/optional-field/optionalChangeRebaser.test.ts index 3286a956810c..5aa2b8216786 100644 --- a/packages/dds/tree/src/test/feature-libraries/optional-field/optionalChangeRebaser.test.ts +++ b/packages/dds/tree/src/test/feature-libraries/optional-field/optionalChangeRebaser.test.ts @@ -525,9 +525,8 @@ function runSingleEditRebaseAxiomSuite(initialState: OptionalFieldTestState) { it(`${name} ○ ${name}⁻¹ === ε`, () => { const inv = invertWrapped(change, tag1, true); const actual = composeWrapped(change, tagRollbackInverse(inv, tag1, change.revision)); - const delta = toDeltaWrapped(makeAnonChange(actual)).local; - assert(delta !== undefined); - assert.equal(isDeltaVisible(delta), false); + const delta = toDeltaWrapped(makeAnonChange(actual)); + assert.equal(isDeltaVisible(delta.local), false); }); } }); @@ -541,9 +540,8 @@ function runSingleEditRebaseAxiomSuite(initialState: OptionalFieldTestState) { change.revision, ); const actual = composeWrapped(inv, change); - const delta = toDeltaWrapped(makeAnonChange(actual)).local; - assert(delta !== undefined); - assert.equal(isDeltaVisible(delta), false); + const delta = toDeltaWrapped(makeAnonChange(actual)); + assert.equal(isDeltaVisible(delta.local), false); }); } }); diff --git a/packages/dds/tree/src/test/feature-libraries/sequence-field/utils.ts b/packages/dds/tree/src/test/feature-libraries/sequence-field/utils.ts index c85929c738d3..3c7566c25a48 100644 --- a/packages/dds/tree/src/test/feature-libraries/sequence-field/utils.ts +++ b/packages/dds/tree/src/test/feature-libraries/sequence-field/utils.ts @@ -12,7 +12,6 @@ import { type ChangeAtomId, type ChangeAtomIdMap, type ChangesetLocalId, - type DeltaRoot, type RevisionInfo, type RevisionMetadataSource, type RevisionTag, @@ -20,7 +19,6 @@ import { makeAnonChange, mapTaggedChange, revisionMetadataSourceFromInfo, - rootFieldKey, tagChange, tagRollbackInverse, } from "../../../core/index.js"; @@ -30,6 +28,7 @@ import { type CrossFieldManager, type CrossFieldQuerySet, CrossFieldTarget, + type FieldChangeDelta, type NodeId, type RebaseRevisionMetadata, setInCrossFieldMap, @@ -74,7 +73,7 @@ import { tryGetFromNestedMap, } from "../../../util/index.js"; import { - assertDeltaEqual, + assertFieldChangesEqual, assertIsSessionId, defaultRevInfosFromChanges, defaultRevisionMetadataFromChanges, @@ -490,17 +489,12 @@ export function invert( } export function checkDeltaEquality(actual: SF.Changeset, expected: SF.Changeset) { - assertDeltaEqual(toDelta(actual), toDelta(expected)); + assertFieldChangesEqual(toDelta(actual), toDelta(expected)); } -export function toDelta(change: SF.Changeset): DeltaRoot { +export function toDelta(change: SF.Changeset): FieldChangeDelta { deepFreeze(change); - const { local, global, rename } = SF.sequenceFieldToDelta(change, TestNodeId.deltaFromChild); - return { - fields: local === undefined ? new Map([]) : new Map([[rootFieldKey, local]]), - global, - rename, - }; + return SF.sequenceFieldToDelta(change, TestNodeId.deltaFromChild); } export function toDeltaWrapped(change: WrappedChange) { diff --git a/packages/dds/tree/src/test/testChange.spec.ts b/packages/dds/tree/src/test/testChange.spec.ts index 72b6e2a76bfd..9fe642d64cec 100644 --- a/packages/dds/tree/src/test/testChange.spec.ts +++ b/packages/dds/tree/src/test/testChange.spec.ts @@ -74,14 +74,7 @@ describe("TestChange", () => { const tag = mintRevisionTag(); const delta = TestChange.toDelta(tagChange(change1, tag)); const field: FieldKey = brand("testIntentions"); - const expected = new Map([ - [ - field, - { - local: [{ count: 2 }, { count: 3 }], - }, - ], - ]); + const expected = new Map([[field, [{ count: 2 }, { count: 3 }]]]); assert.deepEqual(delta, expected); assert.deepEqual( diff --git a/packages/dds/tree/src/test/tree/visitDelta.spec.ts b/packages/dds/tree/src/test/tree/visitDelta.spec.ts index 5453d1e8b154..eb3acbcc1f95 100644 --- a/packages/dds/tree/src/test/tree/visitDelta.spec.ts +++ b/packages/dds/tree/src/test/tree/visitDelta.spec.ts @@ -461,8 +461,6 @@ describe("visitDelta", () => { destroy: [{ id: node1, count: 1 }], }; const expected: VisitScript = [ - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field0], ["enterNode", 0], ["enterField", fooKey], @@ -470,8 +468,6 @@ describe("visitDelta", () => { ["exitField", fooKey], ["exitNode", 0], ["exitField", field0], - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field0], ["enterNode", 0], ["enterField", fooKey], @@ -509,13 +505,9 @@ describe("visitDelta", () => { }; const expected: VisitScript = [ ["create", [content], field0], - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field0], ["detach", { start: 0, end: 1 }, field1], ["exitField", field0], - ["enterField", rootKey], - ["exitField", rootKey], ["destroy", field1, 1], ]; testDeltaVisit(delta, expected, index); @@ -748,13 +740,9 @@ describe("visitDelta", () => { }; const expected: VisitScript = [ ["create", [content], field0], - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field0], ["detach", { start: 0, end: 1 }, field1], ["exitField", field0], - ["enterField", rootKey], - ["exitField", rootKey], ]; testDeltaVisit(delta, expected, index); assert.deepEqual(Array.from(index.entries()), [{ id: { minor: 43 }, root: 1 }]); @@ -779,8 +767,6 @@ describe("visitDelta", () => { }; const expected: VisitScript = [ ["create", [content], field0], // field0: buildId - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field0], ["enterNode", 0], ["enterField", barKey], @@ -791,8 +777,6 @@ describe("visitDelta", () => { ["enterField", field0], ["detach", { start: 0, end: 1 }, field2], // field2: detachId ["exitField", field0], - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field2], ["enterNode", 0], ["enterField", barKey], @@ -870,8 +854,6 @@ describe("visitDelta", () => { }; const delta = { global: [modify] }; const expected: VisitScript = [ - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field0], ["enterNode", 0], ["enterField", fooKey], @@ -879,8 +861,6 @@ describe("visitDelta", () => { ["exitField", fooKey], ["exitNode", 0], ["exitField", field0], - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field0], ["enterNode", 0], ["enterField", fooKey], @@ -952,13 +932,9 @@ describe("visitDelta", () => { }; const delta = { rename: [restore] }; const expected: VisitScript = [ - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field0], ["detach", { start: 0, end: 1 }, field1], ["exitField", field0], - ["enterField", rootKey], - ["exitField", rootKey], ]; testDeltaVisit(delta, expected, index); assert.deepEqual(Array.from(index.entries()), [{ id: { minor: 42 }, root: 1 }]); @@ -985,16 +961,12 @@ describe("visitDelta", () => { }; const expected: VisitScript = [ ["create", [content], field1], // field1: buildId - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field0], // field0: node1 ["detach", { start: 0, end: 1 }, field2], // field2: detachId ["exitField", field0], ["enterField", field1], ["detach", { start: 0, end: 1 }, field3], // field3: node1 ["exitField", field1], - ["enterField", rootKey], - ["exitField", rootKey], ]; testDeltaVisit(delta, expected, index); assert.deepEqual(Array.from(index.entries()), [ @@ -1083,17 +1055,13 @@ describe("visitDelta", () => { const buildId = { minor: 43 }; const expected: VisitScript = [ ["create", [content], field0], - ["enterField", rootKey], ["create", [content], field1], - ["exitField", rootKey], ["enterField", field1], ["enterNode", 0], ["enterField", fooKey], ["exitField", fooKey], ["exitNode", 0], ["exitField", field1], - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field1], ["enterNode", 0], ["enterField", fooKey], @@ -1133,13 +1101,9 @@ describe("visitDelta", () => { rename: [rename], }; const expected: VisitScript = [ - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field0], ["detach", { start: 0, end: 1 }, field1], ["exitField", field0], - ["enterField", rootKey], - ["exitField", rootKey], ]; const revision = mintRevisionTag(); testDeltaVisit(delta, expected, index, revision); @@ -1329,20 +1293,11 @@ describe("visitDelta", () => { rename: [rename], }; const expected: VisitScript = cycle - ? [ - ["enterField", rootKey], - ["exitField", rootKey], - ["enterField", rootKey], - ["exitField", rootKey], - ] + ? [] : [ - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field0], ["detach", { start: 0, end: 1 }, field1], ["exitField", field0], - ["enterField", rootKey], - ["exitField", rootKey], ]; testDeltaVisit(delta, expected, index); assert.deepEqual(Array.from(index.entries()), [{ id: end, root: cycle ? 0 : 1 }]); @@ -1371,16 +1326,12 @@ describe("visitDelta", () => { ][ordering - 1], }; const expected: VisitScript = [ - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field0], ["detach", { start: 0, end: 1 }, field1], ["exitField", field0], ["enterField", field1], ["detach", { start: 0, end: 1 }, field2], ["exitField", field1], - ["enterField", rootKey], - ["exitField", rootKey], ]; testDeltaVisit(delta, expected, index); assert.deepEqual(Array.from(index.entries()), [{ id: end, root: 2 }]); @@ -1418,8 +1369,6 @@ describe("visitDelta", () => { ][ordering - 1], }; const expected: VisitScript = [ - ["enterField", rootKey], - ["exitField", rootKey], ["enterField", field0], ["detach", { start: 0, end: 1 }, field1], ["exitField", field0], @@ -1429,8 +1378,6 @@ describe("visitDelta", () => { ["enterField", field2], ["detach", { start: 0, end: 1 }, field3], ["exitField", field2], - ["enterField", rootKey], - ["exitField", rootKey], ]; const index = makeDetachedFieldIndex("", testRevisionTagCodec, testIdCompressor); index.createEntry(pointA); diff --git a/packages/dds/tree/src/test/utils.ts b/packages/dds/tree/src/test/utils.ts index e40fc8a1803a..7f078023da24 100644 --- a/packages/dds/tree/src/test/utils.ts +++ b/packages/dds/tree/src/test/utils.ts @@ -463,7 +463,7 @@ export function spyOnMethod( /** * Determines whether or not the given delta has a visible impact on the document tree. */ -export function isDeltaVisible(fieldChanges: DeltaFieldChanges): boolean { +export function isDeltaVisible(fieldChanges: DeltaFieldChanges | undefined): boolean { for (const mark of fieldChanges ?? []) { if (mark.attach !== undefined || mark.detach !== undefined) { return true; From 15edaf5f2374eb68eec98322a6bcc36ba252a6a7 Mon Sep 17 00:00:00 2001 From: Jenn Date: Wed, 15 Jan 2025 14:13:58 -0800 Subject: [PATCH 10/16] Update packages/dds/tree/src/core/tree/delta.ts Co-authored-by: yann-achard-MS <97201204+yann-achard-MS@users.noreply.github.com> --- packages/dds/tree/src/core/tree/delta.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/dds/tree/src/core/tree/delta.ts b/packages/dds/tree/src/core/tree/delta.ts index 072353489746..fd4382929a98 100644 --- a/packages/dds/tree/src/core/tree/delta.ts +++ b/packages/dds/tree/src/core/tree/delta.ts @@ -108,7 +108,7 @@ export interface Root { */ readonly global?: readonly DetachedNodeChanges[]; /** - * Detached whose associated ID needs to be updated. + * Detached roots whose associated ID needs to be updated. * The ordering has no significance. * Note that the renames may need to be performed in a specific order to avoid collisions. * This ordering problem is left to the consumer of this format. From 8f5d8e538c16ca9008332db3c583637d3f36131e Mon Sep 17 00:00:00 2001 From: Jenn Date: Wed, 15 Jan 2025 22:31:29 +0000 Subject: [PATCH 11/16] revert undefined change --- .../modular-schema/fieldChangeHandler.ts | 2 +- .../modular-schema/genericFieldKind.ts | 5 +--- .../modular-schema/modularChangeFamily.ts | 25 ++++++++++--------- .../optional-field/optionalField.ts | 2 +- .../sequence-field/sequenceFieldToDelta.ts | 2 +- .../types/validateTreePrevious.generated.ts | 18 ------------- 6 files changed, 17 insertions(+), 37 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts b/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts index 9fd5fdbd9c93..55f4b5849218 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/fieldChangeHandler.ts @@ -210,7 +210,7 @@ export interface FieldEditor { * The `index` represents the index of the child node in the input context. * The `index` should be `undefined` iff the child node does not exist in the input context (e.g., an inserted node). */ -export type ToDelta = (child: NodeId) => DeltaFieldMap | undefined; +export type ToDelta = (child: NodeId) => DeltaFieldMap; /** */ diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts b/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts index 6f44dfc678ab..f5919f8e2ed8 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts @@ -55,10 +55,7 @@ export const genericChangeHandler: FieldChangeHandler = { markList.push({ count: offset }); nodeIndex = index; } - const childDelta = deltaFromChild(nodeChange); - if (childDelta !== undefined) { - markList.push({ count: 1, fields: childDelta }); - } + markList.push({ count: 1, fields: deltaFromChild(nodeChange) }); nodeIndex += 1; } return { local: markList }; diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts index 2aad41a03046..4d18e9be858c 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts @@ -2087,17 +2087,18 @@ function intoDeltaImpl( fieldChange.change, (childChange) => { const nodeChange = nodeChangeFromId(nodeChanges, childChange); - const nodeChangeDelta = deltaFromNodeChange(nodeChange, nodeChanges, fieldKinds); - if (nodeChangeDelta !== undefined) { - const [nodeFieldChanges, nodeGlobals, nodeRenames] = nodeChangeDelta; - if (nodeGlobals.length > 0) { - nodeGlobals.forEach((c) => global.push(c)); - } - if (nodeRenames.length > 0) { - nodeRenames.forEach((r) => rename.push(r)); - } - return nodeFieldChanges; + const [nodeFieldChanges, nodeGlobals, nodeRenames] = deltaFromNodeChange( + nodeChange, + nodeChanges, + fieldKinds, + ); + if (nodeGlobals.length > 0) { + nodeGlobals.forEach((c) => global.push(c)); } + if (nodeRenames.length > 0) { + nodeRenames.forEach((r) => rename.push(r)); + } + return nodeFieldChanges; }, ); if (fieldChanges !== undefined && fieldChanges.length > 0) { @@ -2117,12 +2118,12 @@ function deltaFromNodeChange( change: NodeChangeset, nodeChanges: ChangeAtomIdBTree, fieldKinds: ReadonlyMap, -): [DeltaFieldMap, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]] | undefined { +): [DeltaFieldMap, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]] { if (change.fieldChanges !== undefined) { return intoDeltaImpl(change.fieldChanges, nodeChanges, fieldKinds); } // TODO: update the API to allow undefined to be returned here - return undefined; + return [new Map(), [], []]; } /** diff --git a/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts b/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts index 1500a7bea80f..4d14c23ee9d4 100644 --- a/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts +++ b/packages/dds/tree/src/feature-libraries/optional-field/optionalField.ts @@ -685,7 +685,7 @@ export function optionalFieldIntoDelta( const globals: DeltaDetachedNodeChanges[] = []; for (const [id, childChange] of change.childChanges) { const childDelta = deltaFromChild(childChange); - if (id !== "self" && childDelta !== undefined) { + if (id !== "self") { const fields = childDelta; globals.push({ id: { major: id.revision, minor: id.localId }, diff --git a/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts b/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts index 6ce6753edce5..0d6ebaad39c3 100644 --- a/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts +++ b/packages/dds/tree/src/feature-libraries/sequence-field/sequenceFieldToDelta.ts @@ -40,7 +40,7 @@ export function sequenceFieldToDelta( const changes = mark.changes; if (changes !== undefined) { const nestedDelta = deltaFromChild(changes); - if (nestedDelta !== undefined && nestedDelta.size > 0) { + if (nestedDelta.size > 0) { if (inputCellId === undefined) { deltaMark.fields = nestedDelta; } else { diff --git a/packages/dds/tree/src/test/types/validateTreePrevious.generated.ts b/packages/dds/tree/src/test/types/validateTreePrevious.generated.ts index 56a8c6f90fd3..14710140a89f 100644 --- a/packages/dds/tree/src/test/types/validateTreePrevious.generated.ts +++ b/packages/dds/tree/src/test/types/validateTreePrevious.generated.ts @@ -339,24 +339,6 @@ declare type old_as_current_for_Interface_NodeInDocumentConstraint = requireAssi */ declare type current_as_old_for_Interface_NodeInDocumentConstraint = requireAssignableTo, TypeOnly> -/* - * Validate backward compatibility by using the current type in place of the old type. - * If this test starts failing, it indicates a change that is not backward compatible. - * To acknowledge the breaking change, add the following to package.json under - * typeValidation.broken: - * "Interface_NodeSchemaMetadata": {"backCompat": false} - */ -declare type current_as_old_for_Interface_NodeSchemaMetadata = requireAssignableTo, TypeOnly> - -/* - * Validate backward compatibility by using the current type in place of the old type. - * If this test starts failing, it indicates a change that is not backward compatible. - * To acknowledge the breaking change, add the following to package.json under - * typeValidation.broken: - * "Interface_NodeSchemaOptions": {"backCompat": false} - */ -declare type current_as_old_for_Interface_NodeSchemaOptions = requireAssignableTo, TypeOnly> - /* * Validate backward compatibility by using the current type in place of the old type. * If this test starts failing, it indicates a change that is not backward compatible. From 2b4f4bcfd2122a5dfb9de4cbd5eb3f7076b84c58 Mon Sep 17 00:00:00 2001 From: Jenn Date: Wed, 15 Jan 2025 23:24:30 +0000 Subject: [PATCH 12/16] fix imports --- packages/dds/tree/src/core/index.ts | 1 - packages/dds/tree/src/core/tree/deltaUtil.ts | 3 --- packages/dds/tree/src/core/tree/index.ts | 1 - packages/dds/tree/src/feature-libraries/index.ts | 1 - packages/dds/tree/src/test/changesetWrapper.ts | 3 ++- .../modular-schema/genericFieldKind.spec.ts | 7 ++----- .../optional-field/optionalChangeRebaser.test.ts | 3 ++- .../sequence-field/sequenceFieldToDelta.test.ts | 11 +++++------ packages/dds/tree/src/test/utils.ts | 3 ++- 9 files changed, 13 insertions(+), 20 deletions(-) diff --git a/packages/dds/tree/src/core/index.ts b/packages/dds/tree/src/core/index.ts index e0fe096710b2..5e414810ce21 100644 --- a/packages/dds/tree/src/core/index.ts +++ b/packages/dds/tree/src/core/index.ts @@ -67,7 +67,6 @@ export { forEachField, type PathRootPrefix, deltaForRootInitialization, - emptyFieldChanges, makeDetachedNodeId, offsetDetachId, emptyDelta, diff --git a/packages/dds/tree/src/core/tree/deltaUtil.ts b/packages/dds/tree/src/core/tree/deltaUtil.ts index 2ef13f83f7b4..97eeb2660b7f 100644 --- a/packages/dds/tree/src/core/tree/deltaUtil.ts +++ b/packages/dds/tree/src/core/tree/deltaUtil.ts @@ -3,7 +3,6 @@ * Licensed under the MIT License. */ -import type { FieldChangeDelta } from "../../feature-libraries/index.js"; import type { Mutable } from "../../util/index.js"; import type { FieldKey } from "../schema-stored/index.js"; @@ -13,8 +12,6 @@ import { rootFieldKey } from "./types.js"; export const emptyDelta: Root = {}; -export const emptyFieldChanges: FieldChangeDelta = {}; - export function isAttachMark(mark: Mark): boolean { return mark.attach !== undefined && mark.detach === undefined; } diff --git a/packages/dds/tree/src/core/tree/index.ts b/packages/dds/tree/src/core/tree/index.ts index 08b1e155995c..c12b921f5ae6 100644 --- a/packages/dds/tree/src/core/tree/index.ts +++ b/packages/dds/tree/src/core/tree/index.ts @@ -104,7 +104,6 @@ export { SparseNode, getDescendant } from "./sparseTree.js"; export { deltaForRootInitialization, - emptyFieldChanges, makeDetachedNodeId, offsetDetachId, emptyDelta, diff --git a/packages/dds/tree/src/feature-libraries/index.ts b/packages/dds/tree/src/feature-libraries/index.ts index 2f0b47d78acb..2001d065df67 100644 --- a/packages/dds/tree/src/feature-libraries/index.ts +++ b/packages/dds/tree/src/feature-libraries/index.ts @@ -43,7 +43,6 @@ export { ModularEditBuilder, type FieldEditDescription as EditDescription, type FieldChangeHandler, - type FieldChangeDelta, type FieldChangeRebaser, type FieldEditor, type FieldChangeMap, diff --git a/packages/dds/tree/src/test/changesetWrapper.ts b/packages/dds/tree/src/test/changesetWrapper.ts index 1c43152985e6..20969493f3f6 100644 --- a/packages/dds/tree/src/test/changesetWrapper.ts +++ b/packages/dds/tree/src/test/changesetWrapper.ts @@ -15,7 +15,6 @@ import { taggedOptAtomId, } from "../core/index.js"; import type { - FieldChangeDelta, NodeChangeComposer, NodeChangePruner, NodeChangeRebaser, @@ -31,6 +30,8 @@ import { tryGetFromNestedMap, } from "../util/index.js"; import { TestChange } from "./testChange.js"; +// eslint-disable-next-line import/no-internal-modules +import type { FieldChangeDelta } from "../feature-libraries/modular-schema/fieldChangeHandler.js"; export interface ChangesetWrapper { fieldChange: T; diff --git a/packages/dds/tree/src/test/feature-libraries/modular-schema/genericFieldKind.spec.ts b/packages/dds/tree/src/test/feature-libraries/modular-schema/genericFieldKind.spec.ts index 1f22faedf86b..4aaf761db50a 100644 --- a/packages/dds/tree/src/test/feature-libraries/modular-schema/genericFieldKind.spec.ts +++ b/packages/dds/tree/src/test/feature-libraries/modular-schema/genericFieldKind.spec.ts @@ -6,11 +6,7 @@ import { strict as assert } from "node:assert"; import type { SessionId } from "@fluidframework/id-compressor"; -import type { - GenericChangeset, - CrossFieldManager, - FieldChangeDelta, -} from "../../../feature-libraries/index.js"; +import type { GenericChangeset, CrossFieldManager } from "../../../feature-libraries/index.js"; import { fakeIdAllocator, brand, idAllocatorFromMaxId } from "../../../util/index.js"; import { type EncodingTestData, @@ -21,6 +17,7 @@ import { testRevisionTagCodec, } from "../../utils.js"; import { + type FieldChangeDelta, type FieldChangeEncodingContext, type NodeId, type RebaseRevisionMetadata, diff --git a/packages/dds/tree/src/test/feature-libraries/optional-field/optionalChangeRebaser.test.ts b/packages/dds/tree/src/test/feature-libraries/optional-field/optionalChangeRebaser.test.ts index 5aa2b8216786..c13c135b5529 100644 --- a/packages/dds/tree/src/test/feature-libraries/optional-field/optionalChangeRebaser.test.ts +++ b/packages/dds/tree/src/test/feature-libraries/optional-field/optionalChangeRebaser.test.ts @@ -6,7 +6,7 @@ import { strict as assert } from "node:assert"; import { describeStress, StressMode } from "@fluid-private/stochastic-test-utils"; -import type { CrossFieldManager, FieldChangeDelta } from "../../../feature-libraries/index.js"; +import type { CrossFieldManager } from "../../../feature-libraries/index.js"; import { type ChangeAtomId, type ChangeAtomIdMap, @@ -20,6 +20,7 @@ import { tagRollbackInverse, } from "../../../core/index.js"; import { + type FieldChangeDelta, type NodeChangeComposer, type NodeChangeRebaser, type NodeId, diff --git a/packages/dds/tree/src/test/feature-libraries/sequence-field/sequenceFieldToDelta.test.ts b/packages/dds/tree/src/test/feature-libraries/sequence-field/sequenceFieldToDelta.test.ts index b1a3be331d9a..8377bf1b4cf3 100644 --- a/packages/dds/tree/src/test/feature-libraries/sequence-field/sequenceFieldToDelta.test.ts +++ b/packages/dds/tree/src/test/feature-libraries/sequence-field/sequenceFieldToDelta.test.ts @@ -12,14 +12,9 @@ import { type DeltaMark, type FieldKey, type RevisionTag, - emptyFieldChanges, tagChange, } from "../../../core/index.js"; -import { - type FieldChangeDelta, - type NodeId, - SequenceField as SF, -} from "../../../feature-libraries/index.js"; +import { type NodeId, SequenceField as SF } from "../../../feature-libraries/index.js"; import { brand } from "../../../util/index.js"; import { TestChange } from "../../testChange.js"; import { assertFieldChangesEqual, mintRevisionTag } from "../../utils.js"; @@ -27,6 +22,8 @@ import { TestNodeId } from "../../testNodeId.js"; import { ChangeMaker as Change, MarkMaker as Mark } from "./testEdits.js"; import { inlineRevision, toDelta } from "./utils.js"; import { deepFreeze } from "@fluidframework/test-runtime-utils/internal"; +// eslint-disable-next-line import/no-internal-modules +import type { FieldChangeDelta } from "../../../feature-libraries/modular-schema/fieldChangeHandler.js"; const moveId = brand(4242); const moveId2 = brand(4343); @@ -47,6 +44,8 @@ const childChange1 = TestNodeId.create(nodeId1, TestChange.mint([0], 1)); const childChange1Delta = TestChange.toDelta(tagChange(childChange1.testChange, tag)); const detachId = { major: tag, minor: 42 }; +export const emptyFieldChanges: FieldChangeDelta = {}; + export function testToDelta() { describe("toDelta", () => { it("empty mark list", () => { diff --git a/packages/dds/tree/src/test/utils.ts b/packages/dds/tree/src/test/utils.ts index 7f078023da24..697ce090de9c 100644 --- a/packages/dds/tree/src/test/utils.ts +++ b/packages/dds/tree/src/test/utils.ts @@ -108,7 +108,6 @@ import { MockNodeKeyManager, cursorForMapTreeField, type IDefaultEditBuilder, - type FieldChangeDelta, } from "../feature-libraries/index.js"; // eslint-disable-next-line import/no-internal-modules import { makeSchemaCodec } from "../feature-libraries/schema-index/codec.js"; @@ -159,6 +158,8 @@ import { JsonUnion, cursorToJsonObject, singleJsonCursor } from "./json/index.js // eslint-disable-next-line import/no-internal-modules import type { TreeSimpleContent } from "./feature-libraries/flex-tree/utils.js"; import type { Transactor } from "../shared-tree-core/index.js"; +// eslint-disable-next-line import/no-internal-modules +import type { FieldChangeDelta } from "../feature-libraries/modular-schema/fieldChangeHandler.js"; // Testing utilities From d4d1c03ea46b41ec9a10a6b891e4b73df92cd621 Mon Sep 17 00:00:00 2001 From: Jenn Date: Wed, 15 Jan 2025 23:58:57 +0000 Subject: [PATCH 13/16] pass in global and rename --- .../modular-schema/modularChangeFamily.ts | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts index 4d18e9be858c..b50dcb11f5f7 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts @@ -2009,13 +2009,17 @@ export function intoDelta( ): DeltaRoot { const change = taggedChange.change; const rootDelta: Mutable = {}; + const global: DeltaDetachedNodeChanges[] = []; + const rename: DeltaDetachedNodeRename[] = []; if (!hasConflicts(change)) { // If there are no constraint violations, then tree changes apply. - const [fieldDeltas, global, rename] = intoDeltaImpl( + const fieldDeltas = intoDeltaImpl( change.fieldChanges, change.nodeChanges, fieldKinds, + global, + rename, ); if (fieldDeltas.size > 0) { rootDelta.fields = fieldDeltas; @@ -2073,10 +2077,10 @@ function intoDeltaImpl( change: FieldChangeMap, nodeChanges: ChangeAtomIdBTree, fieldKinds: ReadonlyMap, -): [Map, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]] { + global: DeltaDetachedNodeChanges[], + rename: DeltaDetachedNodeRename[], +): Map { const delta: Map = new Map(); - const global: DeltaDetachedNodeChanges[] = []; - const rename: DeltaDetachedNodeRename[] = []; for (const [field, fieldChange] of change) { const { @@ -2087,17 +2091,13 @@ function intoDeltaImpl( fieldChange.change, (childChange) => { const nodeChange = nodeChangeFromId(nodeChanges, childChange); - const [nodeFieldChanges, nodeGlobals, nodeRenames] = deltaFromNodeChange( + const nodeFieldChanges = deltaFromNodeChange( nodeChange, nodeChanges, fieldKinds, + global, + rename, ); - if (nodeGlobals.length > 0) { - nodeGlobals.forEach((c) => global.push(c)); - } - if (nodeRenames.length > 0) { - nodeRenames.forEach((r) => rename.push(r)); - } return nodeFieldChanges; }, ); @@ -2111,19 +2111,21 @@ function intoDeltaImpl( fieldRename.forEach((r) => rename.push(r)); } } - return [delta, global, rename]; + return delta; } function deltaFromNodeChange( change: NodeChangeset, nodeChanges: ChangeAtomIdBTree, fieldKinds: ReadonlyMap, -): [DeltaFieldMap, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]] { + global: DeltaDetachedNodeChanges[], + rename: DeltaDetachedNodeRename[], +): DeltaFieldMap { if (change.fieldChanges !== undefined) { - return intoDeltaImpl(change.fieldChanges, nodeChanges, fieldKinds); + return intoDeltaImpl(change.fieldChanges, nodeChanges, fieldKinds, global, rename); } // TODO: update the API to allow undefined to be returned here - return [new Map(), [], []]; + return new Map(); } /** From e33b07b8db745c31dce9aa8719d02e00201e9fb2 Mon Sep 17 00:00:00 2001 From: Jenn Date: Thu, 16 Jan 2025 00:27:26 +0000 Subject: [PATCH 14/16] cleanup --- .../modular-schema/genericFieldKind.ts | 3 ++- .../modular-schema/modularChangeFamily.ts | 11 ++--------- .../feature-libraries/modular-schema/basicRebasers.ts | 7 ++++--- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts b/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts index f5919f8e2ed8..254984d483f6 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/genericFieldKind.ts @@ -15,6 +15,7 @@ import { type IdAllocator, fail } from "../../util/index.js"; import { assert } from "@fluidframework/core-utils/internal"; import type { CrossFieldManager } from "./crossFieldQueries.js"; import type { + FieldChangeDelta, FieldChangeHandler, NestedChangesIndices, NodeChangeComposer, @@ -46,7 +47,7 @@ export const genericChangeHandler: FieldChangeHandler = { return newGenericChangeset([[index, change]]); }, }, - intoDelta: (change: GenericChangeset, deltaFromChild: ToDelta) => { + intoDelta: (change: GenericChangeset, deltaFromChild: ToDelta): FieldChangeDelta => { let nodeIndex = 0; const markList: DeltaMark[] = []; for (const [index, nodeChange] of change.entries()) { diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts index b50dcb11f5f7..077e8bc0fbdd 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts @@ -2089,16 +2089,9 @@ function intoDeltaImpl( rename: fieldRename, } = getChangeHandler(fieldKinds, fieldChange.fieldKind).intoDelta( fieldChange.change, - (childChange) => { + (childChange): DeltaFieldMap => { const nodeChange = nodeChangeFromId(nodeChanges, childChange); - const nodeFieldChanges = deltaFromNodeChange( - nodeChange, - nodeChanges, - fieldKinds, - global, - rename, - ); - return nodeFieldChanges; + return deltaFromNodeChange(nodeChange, nodeChanges, fieldKinds, global, rename); }, ); if (fieldChanges !== undefined && fieldChanges.length > 0) { diff --git a/packages/dds/tree/src/test/feature-libraries/modular-schema/basicRebasers.ts b/packages/dds/tree/src/test/feature-libraries/modular-schema/basicRebasers.ts index 8bc3c0cf8e08..1bc78ffc252f 100644 --- a/packages/dds/tree/src/test/feature-libraries/modular-schema/basicRebasers.ts +++ b/packages/dds/tree/src/test/feature-libraries/modular-schema/basicRebasers.ts @@ -16,7 +16,7 @@ import { referenceFreeFieldChangeRebaser, // eslint-disable-next-line import/no-internal-modules } from "../../../feature-libraries/modular-schema/index.js"; -import { fail } from "../../../util/index.js"; +import { fail, type Mutable } from "../../../util/index.js"; import { makeValueCodec } from "../../codec/index.js"; /** @@ -83,14 +83,15 @@ export const valueHandler = { editor: { buildChildChange: (index, change) => fail("Child changes not supported") }, intoDelta: (change): FieldChangeDelta => { + const delta: Mutable = {}; if (change !== 0) { // We use the new and old numbers as the node ids. // These would have no real meaning to a delta consumer, but these delta are only used for testing. const detach = makeDetachedNodeId(undefined, change.old); const attach = makeDetachedNodeId(undefined, change.new); - return { local: [{ count: 1, attach, detach }] }; + delta.local = [{ count: 1, attach, detach }]; } - return {}; + return delta; }, relevantRemovedRoots: (change) => [], From 4ea4c787ebfee4bd2d7400d3e85fbe4f7a401b61 Mon Sep 17 00:00:00 2001 From: Jenn Date: Wed, 15 Jan 2025 16:28:13 -0800 Subject: [PATCH 15/16] Update packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts Co-authored-by: yann-achard-MS <97201204+yann-achard-MS@users.noreply.github.com> --- .../modular-schema/modularChangeFamily.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts index 077e8bc0fbdd..aeeaaf428cb2 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts @@ -2097,12 +2097,8 @@ function intoDeltaImpl( if (fieldChanges !== undefined && fieldChanges.length > 0) { delta.set(field, fieldChanges); } - if (fieldGlobal !== undefined) { - fieldGlobal.forEach((c) => global.push(c)); - } - if (fieldRename !== undefined) { - fieldRename.forEach((r) => rename.push(r)); - } + fieldGlobal?.forEach((c) => global.push(c)); + fieldRename?.forEach((r) => rename.push(r)); } return delta; } From e4fc9985f8a2a21d746c78e6951a34e4563d973a Mon Sep 17 00:00:00 2001 From: Jenn Date: Thu, 16 Jan 2025 17:31:59 +0000 Subject: [PATCH 16/16] fix build --- .../types/validateTreePrevious.generated.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/dds/tree/src/test/types/validateTreePrevious.generated.ts b/packages/dds/tree/src/test/types/validateTreePrevious.generated.ts index 14710140a89f..56a8c6f90fd3 100644 --- a/packages/dds/tree/src/test/types/validateTreePrevious.generated.ts +++ b/packages/dds/tree/src/test/types/validateTreePrevious.generated.ts @@ -339,6 +339,24 @@ declare type old_as_current_for_Interface_NodeInDocumentConstraint = requireAssi */ declare type current_as_old_for_Interface_NodeInDocumentConstraint = requireAssignableTo, TypeOnly> +/* + * Validate backward compatibility by using the current type in place of the old type. + * If this test starts failing, it indicates a change that is not backward compatible. + * To acknowledge the breaking change, add the following to package.json under + * typeValidation.broken: + * "Interface_NodeSchemaMetadata": {"backCompat": false} + */ +declare type current_as_old_for_Interface_NodeSchemaMetadata = requireAssignableTo, TypeOnly> + +/* + * Validate backward compatibility by using the current type in place of the old type. + * If this test starts failing, it indicates a change that is not backward compatible. + * To acknowledge the breaking change, add the following to package.json under + * typeValidation.broken: + * "Interface_NodeSchemaOptions": {"backCompat": false} + */ +declare type current_as_old_for_Interface_NodeSchemaOptions = requireAssignableTo, TypeOnly> + /* * Validate backward compatibility by using the current type in place of the old type. * If this test starts failing, it indicates a change that is not backward compatible.