Skip to content

Commit

Permalink
Merge-Tree and SharedString ISegment Deprecations (#23323)
Browse files Browse the repository at this point in the history
The current ISegment interface over-exposes a number of properties which
do not have an external use case, and any external usage could result in
damage to the underlying merge-tree including data corruption.

The only use case that will continue to be supported is determining if a
segment is removed. For this purpose we've add the following `function
segmentIsRemoved(segment: ISegment): boolean`

For example, checking if a segment is not removed would change as
follows:
``` diff
- if(segment.removedSeq === undefined){
+ if(!segmentIsRemoved(segment)){
```

The following properties are deprecated on ISegment and its
implementations:
- clientId
- index
- localMovedSeq
- localRefs
- localRemovedSeq
- localSeq
- movedClientsIds
- movedSeq
- movedSeqs
- ordinal
- removedClientIds
- removedSeq
- seq
- wasMovedOnInsert


Additionally, the following types are also deprecated, and will become
internal:
- IMergeNodeCommon
- IMoveInfo
- IRemovalInfo
- LocalReferenceCollection

---------

Co-authored-by: Alex Villarreal <716334+alexvy86@users.noreply.github.com>
Co-authored-by: Abram Sanderson <Abram.sanderson@gmail.com>
3 people authored Dec 13, 2024
1 parent f7e9c40 commit e8762e3
Showing 10 changed files with 259 additions and 30 deletions.
41 changes: 41 additions & 0 deletions .changeset/real-grapes-kick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
"@fluidframework/merge-tree": minor
"@fluidframework/sequence": minor
---
---
"section": deprecation
---

Merge-Tree and SharedString ISegment Deprecations

The current ISegment interface over-exposes a number of properties which do not have an external use case, and any external usage could result in damage to the underlying merge-tree including data corruption.

The only use case that will continue to be supported is determining if a segment is removed. For this purpose we've added the free function `segmentIsRemoved(segment: ISegment): boolean`.

For example, checking if a segment is not removed would change as follows:
``` diff
- if(segment.removedSeq === undefined){
+ if(!segmentIsRemoved(segment)){
```

The following properties are deprecated on ISegment and its implementations:
- clientId
- index
- localMovedSeq
- localRefs
- localRemovedSeq
- localSeq
- movedClientsIds
- movedSeq
- movedSeqs
- ordinal
- removedClientIds
- removedSeq
- seq
- wasMovedOnInsert

Additionally, the following types are also deprecated, and will become internal (i.e. users of the Fluid Framework will not have access to them):
- IMergeNodeCommon
- IMoveInfo
- IRemovalInfo
- LocalReferenceCollection
11 changes: 7 additions & 4 deletions examples/data-objects/webflow/src/view/layout.ts
Original file line number Diff line number Diff line change
@@ -7,7 +7,10 @@
import assert from "assert";

import { EventEmitter } from "@fluid-example/example-utils";
import { MergeTreeMaintenanceType } from "@fluidframework/merge-tree/internal";
import {
MergeTreeMaintenanceType,
segmentIsRemoved,
} from "@fluidframework/merge-tree/internal";
import {
ISegment,
LocalReferencePosition,
@@ -309,7 +312,7 @@ export class Layout extends EventEmitter {
);

// Must not request a formatter for a removed segment.
assert.strictEqual(segment.removedSeq, undefined);
assert.strictEqual(segmentIsRemoved(segment), false);

// If we've checkpointed this segment previously, we can potentially reuse our previous state to
// minimize damage to the DOM.
@@ -421,7 +424,7 @@ export class Layout extends EventEmitter {

public nodeToSegment(node: Node): ISegment {
const seg = this.nodeToSegmentMap.get(node);
return seg && (seg.removedSeq === undefined ? seg : undefined);
return seg && (!segmentIsRemoved(seg) ? seg : undefined);
}

public segmentAndOffsetToNodeAndOffset(segment: ISegment, offset: number) {
@@ -609,7 +612,7 @@ export class Layout extends EventEmitter {

// If the segment was removed, promptly remove any DOM nodes it emitted.
for (const { segment } of e.ranges) {
if (segment.removedSeq) {
if (segmentIsRemoved(segment)) {
this.removeSegment(segment);
}
}
69 changes: 50 additions & 19 deletions packages/dds/merge-tree/api-report/merge-tree.legacy.alpha.api.md
Original file line number Diff line number Diff line change
@@ -27,7 +27,7 @@ export abstract class BaseSegment implements ISegment {
cachedLength: number;
// (undocumented)
canAppend(segment: ISegment): boolean;
// (undocumented)
// @deprecated (undocumented)
clientId: number;
// (undocumented)
abstract clone(): ISegment;
@@ -37,33 +37,33 @@ export abstract class BaseSegment implements ISegment {
protected abstract createSplitSegmentAt(pos: number): BaseSegment | undefined;
// (undocumented)
hasProperty(key: string): boolean;
// (undocumented)
// @deprecated (undocumented)
index: number;
// (undocumented)
isLeaf(): this is ISegment;
// (undocumented)
// @deprecated (undocumented)
localMovedSeq?: number;
// (undocumented)
// @deprecated (undocumented)
localRefs?: LocalReferenceCollection;
// (undocumented)
// @deprecated (undocumented)
localRemovedSeq?: number;
// (undocumented)
// @deprecated (undocumented)
localSeq?: number;
// (undocumented)
// @deprecated (undocumented)
movedClientIds?: number[];
// (undocumented)
// @deprecated (undocumented)
movedSeq?: number;
// (undocumented)
// @deprecated (undocumented)
movedSeqs?: number[];
// (undocumented)
// @deprecated (undocumented)
ordinal: string;
// (undocumented)
properties?: PropertySet;
// (undocumented)
// @deprecated (undocumented)
removedClientIds?: number[];
// (undocumented)
// @deprecated (undocumented)
removedSeq?: number;
// (undocumented)
// @deprecated (undocumented)
seq: number;
// (undocumented)
splitAt(pos: number): ISegment | undefined;
@@ -73,7 +73,7 @@ export abstract class BaseSegment implements ISegment {
readonly trackingCollection: TrackingGroupCollection;
// (undocumented)
abstract readonly type: string;
// (undocumented)
// @deprecated (undocumented)
wasMovedOnInsert?: boolean | undefined;
}

@@ -160,7 +160,7 @@ export interface IMarkerDef {
refType?: ReferenceType;
}

// @alpha
// @alpha @deprecated
export interface IMergeNodeCommon {
index: number;
// (undocumented)
@@ -320,7 +320,7 @@ export interface IMergeTreeSegmentDelta {
segment: ISegment;
}

// @alpha
// @alpha @deprecated
export interface IMoveInfo {
localMovedSeq?: number;
movedClientIds: number[];
@@ -345,29 +345,55 @@ export interface IRelativePosition {
offset?: number;
}

// @alpha
// @alpha @deprecated
export interface IRemovalInfo {
localRemovedSeq?: number;
removedClientIds: number[];
removedSeq: number;
}

// @alpha
export interface ISegment extends IMergeNodeCommon, Partial<IRemovalInfo>, Partial<IMoveInfo> {
export interface ISegment {
// (undocumented)
append(segment: ISegment): void;
attribution?: IAttributionCollection<AttributionKey>;
cachedLength: number;
// (undocumented)
canAppend(segment: ISegment): boolean;
// @deprecated
clientId: number;
// (undocumented)
clone(): ISegment;
// @deprecated
readonly endpointType?: "start" | "end";
// @deprecated
index: number;
// (undocumented)
isLeaf(): this is ISegment;
// @deprecated
localMovedSeq?: number;
// @deprecated
localRefs?: LocalReferenceCollection;
// @deprecated
localRemovedSeq?: number;
// @deprecated
localSeq?: number;
// @deprecated
movedClientIds?: number[];
// @deprecated
movedSeq?: number;
// @deprecated
movedSeqs?: number[];
// @deprecated
moveDst?: ReferencePosition;
// @deprecated
ordinal: string;
properties?: PropertySet;
// @deprecated
removedClientIds?: number[];
// @deprecated
removedSeq?: number;
// @deprecated
seq?: number;
// (undocumented)
splitAt(pos: number): ISegment | undefined;
@@ -377,6 +403,8 @@ export interface ISegment extends IMergeNodeCommon, Partial<IRemovalInfo>, Parti
readonly trackingCollection: TrackingGroupCollection;
// (undocumented)
readonly type: string;
// @deprecated
wasMovedOnInsert?: boolean;
}

// @alpha (undocumented)
@@ -399,7 +427,7 @@ export interface ITrackingGroup {
unlink(trackable: Trackable): boolean;
}

// @alpha @sealed
// @alpha @sealed @deprecated
export class LocalReferenceCollection {
[Symbol.iterator](): {
next(): IteratorResult<LocalReferencePosition>;
@@ -568,6 +596,9 @@ export const reservedMarkerIdKey = "markerId";
// @alpha
export function revertMergeTreeDeltaRevertibles(driver: MergeTreeRevertibleDriver, revertibles: MergeTreeDeltaRevertible[]): void;

// @alpha
export function segmentIsRemoved(segment: ISegment): boolean;

// @alpha (undocumented)
export interface SequenceOffsets {
// (undocumented)
9 changes: 6 additions & 3 deletions packages/dds/merge-tree/src/endOfTreeSegment.ts
Original file line number Diff line number Diff line change
@@ -6,10 +6,12 @@
import { assert } from "@fluidframework/core-utils/internal";

import { LocalClientId } from "./constants.js";
// eslint-disable-next-line import/no-deprecated
import { LocalReferenceCollection } from "./localReference.js";
import { MergeTree } from "./mergeTree.js";
import { NodeAction, depthFirstNodeWalk } from "./mergeTreeNodeWalk.js";
import { IRemovalInfo, ISegment, ISegmentLeaf, type MergeBlock } from "./mergeTreeNodes.js";
// eslint-disable-next-line import/no-deprecated
import { ISegment, ISegmentLeaf, type MergeBlock } from "./mergeTreeNodes.js";

/**
* This is a special segment that is not bound or known by the merge tree itself,
@@ -71,6 +73,7 @@ abstract class BaseEndpointSegment {

abstract get ordinal(): string;

// eslint-disable-next-line import/no-deprecated
localRefs?: LocalReferenceCollection;

/*
@@ -99,7 +102,7 @@ const notSupported = (): never => {
/**
* The position immediately prior to the start of the tree
*/
export class StartOfTreeSegment extends BaseEndpointSegment implements ISegment, IRemovalInfo {
export class StartOfTreeSegment extends BaseEndpointSegment implements ISegment {
type: string = "StartOfTreeSegment";
readonly endpointType = "start";

@@ -149,7 +152,7 @@ export class StartOfTreeSegment extends BaseEndpointSegment implements ISegment,
/**
* The position immediately after the end of the tree
*/
export class EndOfTreeSegment extends BaseEndpointSegment implements ISegment, IRemovalInfo {
export class EndOfTreeSegment extends BaseEndpointSegment implements ISegment {
type: string = "EndOfTreeSegment";
readonly endpointType = "end";

1 change: 1 addition & 0 deletions packages/dds/merge-tree/src/index.ts
Original file line number Diff line number Diff line change
@@ -63,6 +63,7 @@ export {
IMergeNodeCommon,
IMoveInfo,
IRemovalInfo,
segmentIsRemoved,
ISegment,
ISegmentAction,
Marker,
1 change: 1 addition & 0 deletions packages/dds/merge-tree/src/localReference.ts
Original file line number Diff line number Diff line change
@@ -228,6 +228,7 @@ export function setValidateRefCount(
*
* @legacy
* @alpha
* @deprecated - This class will be removed in 2.20 with no replacement.
*/
export class LocalReferenceCollection {
public static append(seg1: ISegment, seg2: ISegment): void {
Loading

0 comments on commit e8762e3

Please sign in to comment.