shape ordering: upgrade fractional indexing to use jitter, avoid conf…

…licts (#4312)

We saw several duplicates related to ungrouping of shapes and fractional
indexes having conflicts — it's hard to repro but the theory is that it
has to do (mainly, but not always) with multiplayer conflicts
(especially if you're offline for a while, you come back online and then
you both have the same indices). However, we have seen this in
single-player mode too.

### Here are some of the related bugs:
- #3932
- #4126
- #4210

As I looked into the issue, and looked at the original code it led me
to:
- this issue on the repo:
rocicorp/fractional-indexing#14
- which led to this article:
https://madebyevan.com/algos/crdt-fractional-indexing/
- and this repo implementing it:
https://github.com/TMeerhof/fractional-indexing-jittered

So! This PR now switches tldraw to using the "jittered" version of
fractional indexing to help with these conflicts. It still doesn't
satisfy my curiosity on why we see these issues in single-player mode
(when ungrouping sometimes). But, this should help alleviate this
problem in general, whether single or multiplayer. As noted in the
jitter repo:
> The default character set has a chance of roughly one in 47.000 to
generate the same key for the same input at the cost of making the keys
3 characters longer on average.

Feels definitely like a good tradeoff! I know @SomeHats mentioned about
just allowing for these conflicts but then it sounded like from reading
other people running into this, we'd have to repair things when someone
tried to insert yet another thing between the conflicting items.

### Reference:
- Original fractional index article by dgreensp:
https://observablehq.com/@dgreensp/implementing-fractional-indexing

### Change type

- [x] `bugfix`
- [ ] `improvement`
- [ ] `feature`
- [ ] `api`
- [ ] `other`

### Test plan

- [x] Updated the unit tests, it uses the non-jitter version for
testability

### Release notes

- Shape ordering: upgrade fractional indexing to use jitter, avoid
conflicts

packages/tldraw/src/lib/shapes/arrow/ArrowShapeTool.test.ts

@@ -497,12 +497,12 @@ describe('reparenting issue', () => {
 		const arrow1BoundIndex = editor.getShape(arrow1Id)!.index
 		const arrow2BoundIndex = editor.getShape(arrow2Id)!.index
 		expect(arrow1BoundIndex).toBe('a1V')
-		expect(arrow2BoundIndex).toBe('a1G')
+		expect(arrow2BoundIndex).toBe('a1F')
 
 		// nudge everything around and make sure we all stay in the right order
 		editor.selectAll().nudgeShapes(editor.getSelectedShapeIds(), { x: -1, y: 0 })
 		expect(editor.getShape(arrow1Id)!.index).toBe('a1V')
-		expect(editor.getShape(arrow2Id)!.index).toBe('a1G')
+		expect(editor.getShape(arrow2Id)!.index).toBe('a1F')
 	})
 })
 

packages/utils/api-report.md

@@ -132,25 +132,25 @@ export function getHashForObject(obj: any): string;
 export function getHashForString(string: string): string;
 
 // @public
-export function getIndexAbove(below?: IndexKey | undefined): IndexKey;
+export function getIndexAbove(below?: IndexKey | null | undefined): IndexKey;
 
 // @public
-export function getIndexBelow(above?: IndexKey | undefined): IndexKey;
+export function getIndexBelow(above?: IndexKey | null | undefined): IndexKey;
 
 // @public
-export function getIndexBetween(below: IndexKey | undefined, above: IndexKey | undefined): IndexKey;
+export function getIndexBetween(below: IndexKey | null | undefined, above: IndexKey | null | undefined): IndexKey;
 
 // @public
 export function getIndices(n: number, start?: IndexKey): IndexKey[];
 
 // @public
-export function getIndicesAbove(below: IndexKey | undefined, n: number): IndexKey[];
+export function getIndicesAbove(below: IndexKey | null | undefined, n: number): IndexKey[];
 
 // @public
-export function getIndicesBelow(above: IndexKey | undefined, n: number): IndexKey[];
+export function getIndicesBelow(above: IndexKey | null | undefined, n: number): IndexKey[];
 
 // @public
-export function getIndicesBetween(below: IndexKey | undefined, above: IndexKey | undefined, n: number): IndexKey[];
+export function getIndicesBetween(below: IndexKey | null | undefined, above: IndexKey | null | undefined, n: number): IndexKey[];
 
 // @internal (undocumented)
 export function getOwnProperty<K extends string, V>(obj: Partial<Record<K, V>>, key: K): undefined | V;
@@ -167,7 +167,7 @@ export { Image_2 as Image }
 
 // @public
 export type IndexKey = string & {
-    __orderKey: true;
+    __brand: 'indexKey';
 };
 
 // @public
@@ -409,7 +409,7 @@ export class Timers {
 export { uniq }
 
 // @internal (undocumented)
-export function validateIndexKey(key: string): asserts key is IndexKey;
+export function validateIndexKey(index: string): asserts index is IndexKey;
 
 // @internal (undocumented)
 export function warnDeprecatedGetter(name: string): void;

packages/utils/package.json

@@ -50,6 +50,7 @@
 		}
 	},
 	"dependencies": {
+		"fractional-indexing-jittered": "^0.9.1",
 		"lodash.throttle": "^4.1.1",
 		"lodash.uniq": "^4.5.0"
 	},

packages/utils/src/index.ts

@@ -49,7 +49,6 @@ export {
 	objectMapValues,
 } from './lib/object'
 export { measureAverageDuration, measureCbDuration, measureDuration } from './lib/perf'
-export { type IndexKey } from './lib/reordering/IndexKey'
 export {
 	ZERO_INDEX_KEY,
 	getIndexAbove,
@@ -61,7 +60,8 @@ export {
 	getIndicesBetween,
 	sortByIndex,
 	validateIndexKey,
-} from './lib/reordering/reordering'
+	type IndexKey,
+} from './lib/reordering'
 export { sortById } from './lib/sort'
 export {
 	clearLocalStorage,

packages/utils/src/lib/reordering.ts

@@ -0,0 +1,154 @@
+import { generateNJitteredKeysBetween, generateNKeysBetween } from 'fractional-indexing-jittered'
+
+const SMALLEST_INTEGER = 'A00000000000000000000000000'
+
+const generateKeysFn =
+	process.env.NODE_ENV === 'test' ? generateNKeysBetween : generateNJitteredKeysBetween
+
+/**
+ * A string made up of an integer part followed by a fraction part. The fraction point consists of
+ * zero or more digits with no trailing zeros. Based on
+ * {@link https://observablehq.com/@dgreensp/implementing-fractional-indexing}.
+ *
+ * @public
+ */
+export type IndexKey = string & { __brand: 'indexKey' }
+
+/**
+ * The index key for the first index - 'a0'.
+ * @public
+ */
+export const ZERO_INDEX_KEY = 'a0' as IndexKey
+
+/**
+ * Get the integer part of an index.
+ *
+ * @param index - The index to use.
+ */
+function getIntegerPart(index: string): string {
+	const integerPartLength = getIntegerLength(index.charAt(0))
+	if (integerPartLength > index.length) {
+		throw new Error('invalid index: ' + index)
+	}
+	return index.slice(0, integerPartLength)
+}
+
+/**
+ * Get the length of an integer.
+ *
+ * @param head - The integer to use.
+ */
+function getIntegerLength(head: string): number {
+	if (head >= 'a' && head <= 'z') {
+		return head.charCodeAt(0) - 'a'.charCodeAt(0) + 2
+	} else if (head >= 'A' && head <= 'Z') {
+		return 'Z'.charCodeAt(0) - head.charCodeAt(0) + 2
+	} else {
+		throw new Error('Invalid index key head: ' + head)
+	}
+}
+
+/** @internal */
+export function validateIndexKey(index: string): asserts index is IndexKey {
+	if (index === SMALLEST_INTEGER) {
+		throw new Error('invalid index: ' + index)
+	}
+	// getIntegerPart will throw if the first character is bad,
+	// or the key is too short.  we'd call it to check these things
+	// even if we didn't need the result
+	const i = getIntegerPart(index)
+	const f = index.slice(i.length)
+	if (f.slice(-1) === '0') {
+		throw new Error('invalid index: ' + index)
+	}
+}
+
+/**
+ * Get a number of indices between two indices.
+ * @param below - The index below.
+ * @param above - The index above.
+ * @param n - The number of indices to get.
+ * @public
+ */
+export function getIndicesBetween(
+	below: IndexKey | null | undefined,
+	above: IndexKey | null | undefined,
+	n: number
+) {
+	return generateKeysFn(below ?? null, above ?? null, n) as IndexKey[]
+}
+
+/**
+ * Get a number of indices above an index.
+ * @param below - The index below.
+ * @param n - The number of indices to get.
+ * @public
+ */
+export function getIndicesAbove(below: IndexKey | null | undefined, n: number) {
+	return generateKeysFn(below ?? null, null, n) as IndexKey[]
+}
+
+/**
+ * Get a number of indices below an index.
+ * @param above - The index above.
+ * @param n - The number of indices to get.
+ * @public
+ */
+export function getIndicesBelow(above: IndexKey | null | undefined, n: number) {
+	return generateKeysFn(null, above ?? null, n) as IndexKey[]
+}
+
+/**
+ * Get the index between two indices.
+ * @param below - The index below.
+ * @param above - The index above.
+ * @public
+ */
+export function getIndexBetween(
+	below: IndexKey | null | undefined,
+	above: IndexKey | null | undefined
+) {
+	return generateKeysFn(below ?? null, above ?? null, 1)[0] as IndexKey
+}
+
+/**
+ * Get the index above a given index.
+ * @param below - The index below.
+ * @public
+ */
+export function getIndexAbove(below: IndexKey | null | undefined = null) {
+	return generateKeysFn(below, null, 1)[0] as IndexKey
+}
+
+/**
+ * Get the index below a given index.
+ * @param above - The index above.
+ *  @public
+ */
+export function getIndexBelow(above: IndexKey | null | undefined = null) {
+	return generateKeysFn(null, above, 1)[0] as IndexKey
+}
+
+/**
+ * Get n number of indices, starting at an index.
+ * @param n - The number of indices to get.
+ * @param start -  The index to start at.
+ * @public
+ */
+export function getIndices(n: number, start = 'a1' as IndexKey) {
+	return [start, ...generateKeysFn(start, null, n)] as IndexKey[]
+}
+
+/**
+ * Sort by index.
+ * @param a - An object with an index property.
+ * @param b - An object with an index property.
+ * @public */
+export function sortByIndex<T extends { index: IndexKey }>(a: T, b: T) {
+	if (a.index < b.index) {
+		return -1
+	} else if (a.index > b.index) {
+		return 1
+	}
+	return 0
+}