Skip to content

Commit

Permalink
fix field change handler into delta contract
Browse files Browse the repository at this point in the history
  • Loading branch information
jenn-le authored Jan 8, 2025
1 parent 3753fae commit 2daf01d
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<DeltaDetachedNodeId> => [],
isEmpty: (change: 0) => true,
getNestedChanges: (change: 0) => [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:
Expand Down Expand Up @@ -197,9 +215,7 @@ export interface FieldEditor<TChangeset> {
* 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;

/**
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export const genericChangeHandler: FieldChangeHandler<GenericChangeset> = {
}
nodeIndex += 1;
}
return [markList, [], []];
return { local: markList };
},
relevantRemovedRoots,
isEmpty: (change: GenericChangeset): boolean => change.length === 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
type RelevantRemovedRootsFromChild,
type ToDelta,
type NestedChangesIndices,
type FieldChangeDelta,
} from "../modular-schema/index.js";

import type {
Expand Down Expand Up @@ -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[] = [];
Expand Down Expand Up @@ -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));
}
}
}
}
Expand All @@ -714,7 +708,11 @@ export function optionalFieldIntoDelta(
fieldChanges = [mark];
}

return [fieldChanges, global, rename];
return {
local: fieldChanges,
global,
rename,
};
}

export const optionalChangeHandler: FieldChangeHandler<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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[] = [];
Expand All @@ -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)) {
Expand Down Expand Up @@ -194,5 +186,9 @@ export function sequenceFieldToDelta(
}
fieldChanges.pop();
}
return [fieldChanges, global, rename];
return {
local: fieldChanges,
global,
rename,
};
}

0 comments on commit 2daf01d

Please sign in to comment.