Standardize segment typing based on exposure (#23315)

I merge tree we have three different layers of segment: 1. ISegment - This is the consumer facing type which is exposed via legacy 2. ISegmentInternal - This type is used internally between fluid framework packages that leverage merge-tree 3. ISegmentLeaf - This type is not exported from the merge-tree package. This change standardizes usage in preparation for the upcoming changes which will move some properties between the layers. Doing this change first will reduce the number of lines modified when changes are made. There are not functional changes in this PR, it is all typing, and there are not changes to any types which are exported to customers, so no type test or api markdowns are impacted
microsoft · Dec 12, 2024 · 87f2eef · 87f2eef
1 parent aa49281
commit 87f2eef
Show file tree

Hide file tree

Showing 32 changed files with 151 additions and 140 deletions.
diff --git a/packages/dds/matrix/src/matrix.ts b/packages/dds/matrix/src/matrix.ts
@@ -21,6 +21,7 @@ import {
 	Client,
 	IJSONSegment,
 	IMergeTreeOp,
+	type ISegmentInternal,
 	type LocalReferencePosition,
 	MergeTreeDeltaType,
 	ReferenceType,
@@ -758,7 +759,7 @@ export class SharedMatrix<T = any>
 		ref: LocalReferencePosition,
 		localSeq: number,
 	): number | undefined {
-		const segment = ref.getSegment();
+		const segment: ISegmentInternal | undefined = ref.getSegment();
 		const offset = ref.getOffset();
 		// If the segment that contains the position is removed, then this setCell op should do nothing.
 		if (segment === undefined || offset === undefined || segment.removedSeq !== undefined) {

diff --git a/packages/dds/matrix/src/permutationvector.ts b/packages/dds/matrix/src/permutationvector.ts
@@ -18,6 +18,7 @@ import {
 	IMergeTreeDeltaOpArgs,
 	IMergeTreeMaintenanceCallbackArgs,
 	ISegment,
+	ISegmentInternal,
 	MergeTreeDeltaType,
 	MergeTreeMaintenanceType,
 	type IMergeTreeInsertMsg,
@@ -200,7 +201,7 @@ export class PermutationVector extends Client {
 		pos: number,
 		op: Pick<ISequencedDocumentMessage, "referenceSequenceNumber" | "clientId">,
 	): number | undefined {
-		const { segment, offset } = this.getContainingSegment(pos, {
+		const { segment, offset } = this.getContainingSegment<ISegmentInternal>(pos, {
 			referenceSequenceNumber: op.referenceSequenceNumber,
 			clientId: op.clientId,
 		});

diff --git a/packages/dds/merge-tree/src/MergeTreeTextHelper.ts b/packages/dds/merge-tree/src/MergeTreeTextHelper.ts
@@ -5,7 +5,7 @@
 
 import { IIntegerRange } from "./client.js";
 import { MergeTree } from "./mergeTree.js";
-import { ISegment } from "./mergeTreeNodes.js";
+import { ISegmentLeaf } from "./mergeTreeNodes.js";
 // eslint-disable-next-line import/no-deprecated
 import { IMergeTreeTextHelper, TextSegment } from "./textSegment.js";
 
@@ -56,7 +56,7 @@ export class MergeTreeTextHelper implements IMergeTreeTextHelper {
 }
 
 function gatherText(
-	segment: ISegment,
+	segment: ISegmentLeaf,
 	pos: number,
 	refSeq: number,
 	clientId: number,

diff --git a/packages/dds/merge-tree/src/attributionPolicy.ts b/packages/dds/merge-tree/src/attributionPolicy.ts
@@ -19,6 +19,7 @@ import {
 	IMergeTreeSegmentDelta,
 	MergeTreeMaintenanceType,
 } from "./mergeTreeDeltaCallback.js";
+import type { ISegmentLeaf } from "./mergeTreeNodes.js";
 import { MergeTreeDeltaType } from "./ops.js";
 
 // Note: these thinly wrap MergeTreeDeltaCallback and MergeTreeMaintenanceCallback to provide the client.
@@ -102,7 +103,8 @@ const attributeInsertionOnSegments = (
 	key: AttributionKey,
 ): void => {
 	for (const { segment } of deltaSegments) {
-		if (segment.seq !== undefined) {
+		const seg: ISegmentLeaf = segment;
+		if (seg.seq !== undefined) {
 			segment.attribution?.update(
 				undefined,
 				new AttributionCollection(segment.cachedLength, key),

diff --git a/packages/dds/merge-tree/src/client.ts b/packages/dds/merge-tree/src/client.ts
@@ -396,7 +396,7 @@ export class Client extends TypedEventEmitter<IClientEvents> {
 	): void {
 		let localInserts = 0;
 		let localRemoves = 0;
-		walkAllChildSegments(this._mergeTree.root, (seg) => {
+		walkAllChildSegments(this._mergeTree.root, (seg: ISegmentLeaf) => {
 			if (seg.seq === UnassignedSequenceNumber) {
 				localInserts++;
 			}

diff --git a/packages/dds/merge-tree/src/mergeTreeNodeWalk.ts b/packages/dds/merge-tree/src/mergeTreeNodeWalk.ts
@@ -3,7 +3,7 @@
  * Licensed under the MIT License.
  */
 
-import { type MergeBlock, IMergeNode, ISegment } from "./mergeTreeNodes.js";
+import { type ISegmentLeaf, type MergeBlock, IMergeNode } from "./mergeTreeNodes.js";
 
 export const LeafAction = {
 	Exit: false,
@@ -38,7 +38,7 @@ export function depthFirstNodeWalk(
 	startBlock: MergeBlock,
 	startChild: IMergeNode | undefined,
 	downAction?: (node: IMergeNode) => NodeAction,
-	leafActionOverride?: (seg: ISegment) => LeafAction,
+	leafActionOverride?: (seg: ISegmentLeaf) => LeafAction,
 	upAction?: (block: MergeBlock) => void,
 	forward: boolean = true,
 ): boolean {
@@ -77,7 +77,7 @@ export function depthFirstNodeWalk(
 			for (let i = start.index; i !== -1 && i !== childCount; i += increment) {
 				// the above loop ensures start is a leaf or undefined, so all children
 				// will be leaves if start exits, so the cast is safe
-				if (leafAction(block.children[i] as ISegment) === LeafAction.Exit) {
+				if (leafAction(block.children[i] as ISegmentLeaf) === LeafAction.Exit) {
 					exit = true;
 					break;
 				}
@@ -122,7 +122,7 @@ export function depthFirstNodeWalk(
  */
 export function forwardExcursion(
 	startNode: IMergeNode,
-	leafAction: (seg: ISegment) => boolean | undefined,
+	leafAction: (seg: ISegmentLeaf) => boolean | undefined,
 ): boolean {
 	if (startNode.parent === undefined) {
 		return true;
@@ -145,7 +145,7 @@ export function forwardExcursion(
  */
 export function backwardExcursion(
 	startNode: IMergeNode,
-	leafAction: (seg: ISegment) => boolean | undefined,
+	leafAction: (seg: ISegmentLeaf) => boolean | undefined,
 ): boolean {
 	if (startNode.parent === undefined) {
 		return true;
@@ -171,7 +171,7 @@ export function backwardExcursion(
  */
 export function walkAllChildSegments(
 	startBlock: MergeBlock,
-	leafAction: (segment: ISegment) => boolean | undefined | void,
+	leafAction: (segment: ISegmentLeaf) => boolean | undefined | void,
 ): boolean {
 	if (startBlock.childCount === 0) {
 		return true;

diff --git a/packages/dds/merge-tree/src/mergeTreeNodes.ts b/packages/dds/merge-tree/src/mergeTreeNodes.ts
@@ -322,8 +322,8 @@ export interface ISegmentAction<TClientData> {
  * @internal
  */
 export interface ISegmentChanges {
-	next?: ISegment;
-	replaceCurrent?: ISegment;
+	next?: ISegmentInternal;
+	replaceCurrent?: ISegmentInternal;
 }
 /**
  * @internal
@@ -361,8 +361,12 @@ export interface NodeAction<TClientData> {
  * @internal
  */
 export interface InsertContext {
-	candidateSegment?: ISegment;
-	leaf: (segment: ISegment | undefined, pos: number, ic: InsertContext) => ISegmentChanges;
+	candidateSegment?: ISegmentInternal;
+	leaf: (
+		segment: ISegmentInternal | undefined,
+		pos: number,
+		ic: InsertContext,
+	) => ISegmentChanges;
 	continuePredicate?: (continueFromBlock: MergeBlock) => boolean;
 }
 
@@ -433,7 +437,7 @@ export class MergeBlock implements IMergeNodeCommon {
 	 */
 	public leftmostTiles: Readonly<MapLike<Marker>>;
 
-	isLeaf(): this is ISegment {
+	isLeaf(): this is ISegmentInternal {
 		return false;
 	}
 

diff --git a/packages/dds/merge-tree/src/partialLengths.ts b/packages/dds/merge-tree/src/partialLengths.ts
@@ -14,7 +14,7 @@ import {
 	IMergeNode,
 	IMoveInfo,
 	IRemovalInfo,
-	ISegment,
+	ISegmentLeaf,
 	compareNumbers,
 	seqLTE,
 	toMoveInfo,
@@ -504,7 +504,7 @@ export class PartialSequenceLengths {
 	 */
 	static accumulateMoveOverlapForExisting(
 		segmentLen: number,
-		segment: ISegment,
+		segment: ISegmentLeaf,
 		firstGte: PartialSequenceLength,
 		clientIds: number[],
 	): void {
@@ -537,7 +537,7 @@ export class PartialSequenceLengths {
 	 * segment
 	 */
 	private static getMoveOverlapForExisting(
-		segment: ISegment,
+		segment: ISegmentLeaf,
 		obliterateOverlapLen: number,
 		clientIds: number[],
 	): RedBlackTree<number, IOverlapClient> {
@@ -558,7 +558,7 @@ export class PartialSequenceLengths {
 	}
 
 	private static updatePartialsAfterInsertion(
-		segment: ISegment,
+		segment: ISegmentLeaf,
 		segmentLen: number,
 		remoteObliteratedLen: number | undefined,
 		obliterateOverlapLen: number = segmentLen,
@@ -638,7 +638,7 @@ export class PartialSequenceLengths {
 	 */
 	private static insertSegment(
 		combinedPartialLengths: PartialSequenceLengths,
-		segment: ISegment,
+		segment: ISegmentLeaf,
 		removalInfo?: IRemovalInfo,
 		moveInfo?: IMoveInfo,
 	): void {

diff --git a/packages/dds/merge-tree/src/perspective.ts b/packages/dds/merge-tree/src/perspective.ts
@@ -6,14 +6,14 @@
 import { UnassignedSequenceNumber } from "./constants.js";
 import { type MergeTree } from "./mergeTree.js";
 import { LeafAction, backwardExcursion, forwardExcursion } from "./mergeTreeNodeWalk.js";
-import { seqLTE, type ISegment } from "./mergeTreeNodes.js";
+import { seqLTE, type ISegmentLeaf } from "./mergeTreeNodes.js";
 
 /**
  * Provides a view of a MergeTree from the perspective of a specific client at a specific sequence number.
  */
 export interface Perspective {
-	nextSegment(segment: ISegment, forward?: boolean): ISegment;
-	previousSegment(segment: ISegment): ISegment;
+	nextSegment(segment: ISegmentLeaf, forward?: boolean): ISegmentLeaf;
+	previousSegment(segment: ISegmentLeaf): ISegmentLeaf;
 }
 
 /**
@@ -47,9 +47,9 @@ export class PerspectiveImpl implements Perspective {
 	 * @param forward - The direction to search.
 	 * @returns the next segment in the specified direction, or the start or end of the tree if there is no next segment.
 	 */
-	public nextSegment(segment: ISegment, forward: boolean = true): ISegment {
-		let next: ISegment | undefined;
-		const action = (seg: ISegment): boolean | undefined => {
+	public nextSegment(segment: ISegmentLeaf, forward: boolean = true): ISegmentLeaf {
+		let next: ISegmentLeaf | undefined;
+		const action = (seg: ISegmentLeaf): boolean | undefined => {
 			if (isSegmentPresent(seg, this._seqTime)) {
 				next = seg;
 				return LeafAction.Exit;
@@ -65,7 +65,7 @@ export class PerspectiveImpl implements Perspective {
 	 * @returns the previous segment, or the start of the tree if there is no previous segment.
 	 * @remarks This is a convenient equivalent to calling `nextSegment(segment, false)`.
 	 */
-	public previousSegment(segment: ISegment): ISegment {
+	public previousSegment(segment: ISegmentLeaf): ISegmentLeaf {
 		return this.nextSegment(segment, false);
 	}
 }
@@ -77,7 +77,7 @@ export class PerspectiveImpl implements Perspective {
  * @param localSeq - The latest local sequence number to consider.
  * @returns true iff this segment was removed in the given perspective.
  */
-export function wasRemovedBefore(seg: ISegment, { refSeq, localSeq }: SeqTime): boolean {
+export function wasRemovedBefore(seg: ISegmentLeaf, { refSeq, localSeq }: SeqTime): boolean {
 	if (
 		seg.removedSeq === UnassignedSequenceNumber &&
 		localSeq !== undefined &&
@@ -95,7 +95,7 @@ export function wasRemovedBefore(seg: ISegment, { refSeq, localSeq }: SeqTime):
  * @param localSeq - The latest local sequence number to consider.
  * @returns true iff this segment was moved (aka obliterated) in the given perspective.
  */
-export function wasMovedBefore(seg: ISegment, { refSeq, localSeq }: SeqTime): boolean {
+export function wasMovedBefore(seg: ISegmentLeaf, { refSeq, localSeq }: SeqTime): boolean {
 	if (
 		seg.movedSeq === UnassignedSequenceNumber &&
 		localSeq !== undefined &&
@@ -109,7 +109,7 @@ export function wasMovedBefore(seg: ISegment, { refSeq, localSeq }: SeqTime): bo
 /**
  * See {@link wasRemovedBefore} and {@link wasMovedBefore}.
  */
-export function wasRemovedOrMovedBefore(seg: ISegment, seqTime: SeqTime): boolean {
+export function wasRemovedOrMovedBefore(seg: ISegmentLeaf, seqTime: SeqTime): boolean {
 	return wasRemovedBefore(seg, seqTime) || wasMovedBefore(seg, seqTime);
 }
 
@@ -120,7 +120,7 @@ export function wasRemovedOrMovedBefore(seg: ISegment, seqTime: SeqTime): boolea
  * @returns true iff this segment was inserted before the given perspective,
  * and it was not removed or moved in the given perspective.
  */
-export function isSegmentPresent(seg: ISegment, seqTime: SeqTime): boolean {
+export function isSegmentPresent(seg: ISegmentLeaf, seqTime: SeqTime): boolean {
 	const { refSeq, localSeq } = seqTime;
 	// If seg.seq is undefined, then this segment has existed since minSeq.
 	// It may have been moved or removed since.

diff --git a/packages/dds/merge-tree/src/revertibles.ts b/packages/dds/merge-tree/src/revertibles.ts
@@ -12,12 +12,7 @@ import { LocalReferenceCollection, LocalReferencePosition } from "./localReferen
 import { MergeTree, findRootMergeBlock } from "./mergeTree.js";
 import { IMergeTreeDeltaCallbackArgs } from "./mergeTreeDeltaCallback.js";
 import { depthFirstNodeWalk } from "./mergeTreeNodeWalk.js";
-import {
-	ISegment,
-	ISegmentInternal,
-	toRemovalInfo,
-	type ISegmentLeaf,
-} from "./mergeTreeNodes.js";
+import { toRemovalInfo, type ISegmentLeaf } from "./mergeTreeNodes.js";
 import { ITrackingGroup, Trackable, UnorderedTrackingGroup } from "./mergeTreeTracking.js";
 import { IJSONSegment, MergeTreeDeltaType, ReferenceType } from "./ops.js";
 import { PropertySet, matchProperties } from "./properties.js";
@@ -246,7 +241,7 @@ export function discardMergeTreeDeltaRevertible(
 			t.trackingCollection.unlink(r.trackingGroup);
 			// remove untracked local references
 			if (t.trackingCollection.empty && !t.isLeaf()) {
-				const segment: ISegmentInternal | undefined = t.getSegment();
+				const segment: ISegmentLeaf | undefined = t.getSegment();
 				segment?.localRefs?.removeLocalRef(t);
 			}
 		}
@@ -287,7 +282,7 @@ function revertLocalRemove(
 
 		assert(!tracked.isLeaf(), 0x3f4 /* removes must track local refs */);
 
-		const refSeg: ISegmentInternal | undefined = tracked.getSegment();
+		const refSeg: ISegmentLeaf | undefined = tracked.getSegment();
 		let realPos = mergeTreeWithRevert.referencePositionToLocalPosition(tracked);
 
 		// References which are on EndOfStringSegment don't return detached for pos,
@@ -340,7 +335,7 @@ function revertLocalRemove(
 			insertSegment.parent!,
 			insertSegment,
 			undefined,
-			(seg: ISegmentInternal) => {
+			(seg: ISegmentLeaf) => {
 				if (seg.localRefs?.empty === false) {
 					return seg.localRefs.walkReferences(refHandler, undefined, forward);
 				}
@@ -372,7 +367,7 @@ function revertLocalRemove(
 			tg.link(insertSegment);
 			tg.unlink(tracked);
 		}
-		const segment: ISegmentInternal | undefined = tracked.getSegment();
+		const segment: ISegmentLeaf | undefined = tracked.getSegment();
 		segment?.localRefs?.removeLocalRef(tracked);
 	}
 }
@@ -393,7 +388,7 @@ function revertLocalAnnotate(
 	}
 }
 
-function getPosition(mergeTreeWithRevert: MergeTreeWithRevert, segment: ISegment): number {
+function getPosition(mergeTreeWithRevert: MergeTreeWithRevert, segment: ISegmentLeaf): number {
 	return mergeTreeWithRevert.getPosition(
 		segment,
 		mergeTreeWithRevert.collabWindow.currentSeq,