Skip to content

Commit

Permalink
Clone list of revertibles (#23424)
Browse files Browse the repository at this point in the history
#### Description


[13865](https://dev.azure.com/fluidframework/internal/_workitems/edit/13865/)

This PR adds a helper function that takes in the list of revertibles and
returns the cloned revertibles in order.

---------

Co-authored-by: Jenn <jennle@microsoft.com>
  • Loading branch information
jikim-msft and jenn-le authored Jan 9, 2025
1 parent a9232b2 commit 29dd73a
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 0 deletions.
3 changes: 3 additions & 0 deletions packages/dds/tree/api-report/tree.alpha.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ export interface BranchableTree extends ViewableTree {
rebase(branch: TreeBranchFork): void;
}

// @alpha
export function cloneRevertibles(revertibles: RevertibleAlpha[], targetBranch: TreeBranch): RevertibleAlpha[];

// @public
export enum CommitKind {
Default = 0,
Expand Down
1 change: 1 addition & 0 deletions packages/dds/tree/src/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,4 +218,5 @@ export {
type RevertibleFactory,
type RevertibleAlphaFactory,
type RevertibleAlpha,
cloneRevertibles,
} from "./revertible.js";
28 changes: 28 additions & 0 deletions packages/dds/tree/src/core/revertible.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ export interface RevertibleAlpha extends Revertible {
/**
* Clones the {@link Revertible} to a target branch.
*
* @remarks To clone a group of`RevertibleAlpha`s, use {@link cloneRevertibles}.
*
* @param branch - A target branch to apply the revertible to.
* The target branch must contain the same commit that this revertible is meant to revert, otherwise will throw an error.
* @returns A cloned revertible is independent of the original, meaning disposing of one will not affect the other,
Expand Down Expand Up @@ -96,3 +98,29 @@ export type RevertibleFactory = (
export type RevertibleAlphaFactory = (
onRevertibleDisposed?: (revertible: RevertibleAlpha) => void,
) => RevertibleAlpha;

/**
* Clones a group of revertibles for a target branch.
*
* @throws Error if any revertible is disposed.
* @throws Error if the target branch does not contain the changes that the revertibles are meant to revert.
*
* @param revertibles - Array of revertibles to clone.
* @param targetBranch - The target branch to clone the revertibles for.
* @returns Array of cloned revertibles, maintaining the same order as the input.
*
* @alpha
*/
export function cloneRevertibles(
revertibles: RevertibleAlpha[],
targetBranch: TreeBranch,
): RevertibleAlpha[] {
const disposedRevertible = revertibles.find(
(revertible) => revertible.status === RevertibleStatus.Disposed,
);
if (disposedRevertible !== undefined) {
throw new Error("List of revertible should not contain disposed revertibles.");
}

return revertibles.map((revertible) => revertible.clone(targetBranch));
}
1 change: 1 addition & 0 deletions packages/dds/tree/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export {
type RevertibleFactory,
type RevertibleAlphaFactory,
type RevertibleAlpha,
cloneRevertibles,
} from "./core/index.js";

export type {
Expand Down
72 changes: 72 additions & 0 deletions packages/dds/tree/src/test/shared-tree/undo.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import {
type FieldUpPath,
type Revertible,
cloneRevertibles,
RevertibleStatus,
type UpPath,
rootFieldKey,
Expand Down Expand Up @@ -676,6 +677,77 @@ describe("Undo and redo", () => {
"Error: Unable to revert a revertible that has been disposed.",
);
});

// TODO:#24414: Enable forkable revertibles tests to run on attached/detached mode.
it("clone list of revertibles", () => {
const view = createInitializedView();
const { undoStack } = createTestUndoRedoStacks(view.events);

assert(view.root.child !== undefined);
view.root.child.propertyOne = 256; // 128 -> 256
view.root.child.propertyTwo.itemOne = "newItem"; // "" -> "newItem"

const forkedView = view.fork();

const revertibles = [...undoStack];
const clonedRevertibles = cloneRevertibles(revertibles, forkedView);

assert.equal(clonedRevertibles.length, 2);
assert.equal(forkedView.root.child?.propertyOne, 256);
assert.equal(forkedView.root.child?.propertyTwo.itemOne, "newItem");

assert.equal(clonedRevertibles[0]?.status, RevertibleStatus.Valid);
assert.equal(clonedRevertibles[1]?.status, RevertibleStatus.Valid);

clonedRevertibles.pop()?.revert();
assert.equal(forkedView.root.child?.propertyOne, 256);
assert.equal(forkedView.root.child?.propertyTwo.itemOne, "");

clonedRevertibles.pop()?.revert();
assert.equal(forkedView.root.child?.propertyOne, 128);
});

// TODO:#24414: Enable forkable revertibles tests to run on attached/detached mode.
it("cloning list of disposed revertibles throws error", () => {
const view = createInitializedView();
const { undoStack } = createTestUndoRedoStacks(view.events);

assert(view.root.child !== undefined);
view.root.child.propertyOne = 256; // 128 -> 256
view.root.child.propertyTwo.itemOne = "newItem"; // "" -> "newItem"

const forkedView = view.fork();
const revertibles = [...undoStack];

for (const revertible of undoStack) {
revertible.revert();
assert.equal(revertible.status, RevertibleStatus.Disposed);
}

assert.throws(
() => cloneRevertibles(revertibles, forkedView),
/List of revertible should not contain disposed revertibles./,
);
});

// TODO:#24414: Enable forkable revertibles tests to run on attached/detached mode.
it("cloning list of revertibles between views with different changes throws error", () => {
const viewA = createInitializedView();
const viewB = createInitializedView();

const { undoStack } = createTestUndoRedoStacks(viewA.events);

assert(viewA.root.child !== undefined);
viewA.root.child.propertyOne = 256; // 128 -> 256
viewA.root.child.propertyTwo.itemOne = "newItem"; // "" -> "newItem"

const revertibles = [...undoStack];

assert.throws(
() => cloneRevertibles(revertibles, viewB),
/Cannot clone revertible for a commit that is not present on the given branch./,
);
});
});

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ export interface BranchableTree extends ViewableTree {
rebase(branch: TreeBranchFork): void;
}

// @alpha
export function cloneRevertibles(revertibles: RevertibleAlpha[], targetBranch: TreeBranch): RevertibleAlpha[];

// @public
export enum CommitKind {
Default = 0,
Expand Down

0 comments on commit 29dd73a

Please sign in to comment.