feat: ability to debug the state of fractional indices (#8235)
This commit is contained in:
parent
7b36de0476
commit
d0a380758e
@ -1,5 +1,10 @@
|
|||||||
|
import { ENV } from "../constants";
|
||||||
import type { OrderedExcalidrawElement } from "../element/types";
|
import type { OrderedExcalidrawElement } from "../element/types";
|
||||||
import { orderByFractionalIndex, syncInvalidIndices } from "../fractionalIndex";
|
import {
|
||||||
|
orderByFractionalIndex,
|
||||||
|
syncInvalidIndices,
|
||||||
|
validateFractionalIndices,
|
||||||
|
} from "../fractionalIndex";
|
||||||
import type { AppState } from "../types";
|
import type { AppState } from "../types";
|
||||||
import type { MakeBrand } from "../utility-types";
|
import type { MakeBrand } from "../utility-types";
|
||||||
import { arrayToMap } from "../utils";
|
import { arrayToMap } from "../utils";
|
||||||
@ -72,6 +77,27 @@ export const reconcileElements = (
|
|||||||
|
|
||||||
const orderedElements = orderByFractionalIndex(reconciledElements);
|
const orderedElements = orderByFractionalIndex(reconciledElements);
|
||||||
|
|
||||||
|
if (
|
||||||
|
import.meta.env.DEV ||
|
||||||
|
import.meta.env.MODE === ENV.TEST ||
|
||||||
|
window?.DEBUG_FRACTIONAL_INDICES
|
||||||
|
) {
|
||||||
|
const elements = syncInvalidIndices(
|
||||||
|
// create new instances due to the mutation
|
||||||
|
orderedElements.map((x) => ({ ...x })),
|
||||||
|
);
|
||||||
|
|
||||||
|
validateFractionalIndices(elements, {
|
||||||
|
// throw in dev & test only, to remain functional on `DEBUG_FRACTIONAL_INDICES`
|
||||||
|
shouldThrow: import.meta.env.DEV || import.meta.env.MODE === ENV.TEST,
|
||||||
|
includeBoundTextValidation: true,
|
||||||
|
reconciliationContext: {
|
||||||
|
localElements,
|
||||||
|
remoteElements,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
// de-duplicate indices
|
// de-duplicate indices
|
||||||
syncInvalidIndices(orderedElements);
|
syncInvalidIndices(orderedElements);
|
||||||
|
|
||||||
|
@ -6,6 +6,9 @@ import type {
|
|||||||
OrderedExcalidrawElement,
|
OrderedExcalidrawElement,
|
||||||
} from "./element/types";
|
} from "./element/types";
|
||||||
import { InvalidFractionalIndexError } from "./errors";
|
import { InvalidFractionalIndexError } from "./errors";
|
||||||
|
import { hasBoundTextElement } from "./element/typeChecks";
|
||||||
|
import { getBoundTextElement } from "./element/textElement";
|
||||||
|
import { arrayToMap } from "./utils";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Envisioned relation between array order and fractional indices:
|
* Envisioned relation between array order and fractional indices:
|
||||||
@ -30,17 +33,80 @@ import { InvalidFractionalIndexError } from "./errors";
|
|||||||
* @throws `InvalidFractionalIndexError` if invalid index is detected.
|
* @throws `InvalidFractionalIndexError` if invalid index is detected.
|
||||||
*/
|
*/
|
||||||
export const validateFractionalIndices = (
|
export const validateFractionalIndices = (
|
||||||
indices: (ExcalidrawElement["index"] | undefined)[],
|
elements: readonly ExcalidrawElement[],
|
||||||
|
{
|
||||||
|
shouldThrow = false,
|
||||||
|
includeBoundTextValidation = false,
|
||||||
|
reconciliationContext,
|
||||||
|
}: {
|
||||||
|
shouldThrow: boolean;
|
||||||
|
includeBoundTextValidation: boolean;
|
||||||
|
reconciliationContext?: {
|
||||||
|
localElements: ReadonlyArray<ExcalidrawElement>;
|
||||||
|
remoteElements: ReadonlyArray<ExcalidrawElement>;
|
||||||
|
};
|
||||||
|
},
|
||||||
) => {
|
) => {
|
||||||
|
const errorMessages = [];
|
||||||
|
const stringifyElement = (element: ExcalidrawElement | void) =>
|
||||||
|
`${element?.index}:${element?.id}:${element?.type}:${element?.isDeleted}:${element?.version}:${element?.versionNonce}`;
|
||||||
|
|
||||||
|
const indices = elements.map((x) => x.index);
|
||||||
for (const [i, index] of indices.entries()) {
|
for (const [i, index] of indices.entries()) {
|
||||||
const predecessorIndex = indices[i - 1];
|
const predecessorIndex = indices[i - 1];
|
||||||
const successorIndex = indices[i + 1];
|
const successorIndex = indices[i + 1];
|
||||||
|
|
||||||
if (!isValidFractionalIndex(index, predecessorIndex, successorIndex)) {
|
if (!isValidFractionalIndex(index, predecessorIndex, successorIndex)) {
|
||||||
throw new InvalidFractionalIndexError(
|
errorMessages.push(
|
||||||
`Fractional indices invariant for element has been compromised - ["${predecessorIndex}", "${index}", "${successorIndex}"] [predecessor, current, successor]`,
|
`Fractional indices invariant has been compromised: "${stringifyElement(
|
||||||
|
elements[i - 1],
|
||||||
|
)}", "${stringifyElement(elements[i])}", "${stringifyElement(
|
||||||
|
elements[i + 1],
|
||||||
|
)}"`,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// disabled by default, as we don't fix it
|
||||||
|
if (includeBoundTextValidation && hasBoundTextElement(elements[i])) {
|
||||||
|
const container = elements[i];
|
||||||
|
const text = getBoundTextElement(container, arrayToMap(elements));
|
||||||
|
|
||||||
|
if (text && text.index! <= container.index!) {
|
||||||
|
errorMessages.push(
|
||||||
|
`Fractional indices invariant for bound elements has been compromised: "${stringifyElement(
|
||||||
|
text,
|
||||||
|
)}", "${stringifyElement(container)}"`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (errorMessages.length) {
|
||||||
|
const error = new InvalidFractionalIndexError();
|
||||||
|
const additionalContext = [];
|
||||||
|
|
||||||
|
if (reconciliationContext) {
|
||||||
|
additionalContext.push("Additional reconciliation context:");
|
||||||
|
additionalContext.push(
|
||||||
|
reconciliationContext.localElements.map((x) => stringifyElement(x)),
|
||||||
|
);
|
||||||
|
additionalContext.push(
|
||||||
|
reconciliationContext.remoteElements.map((x) => stringifyElement(x)),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
// report just once and with the stacktrace
|
||||||
|
console.error(
|
||||||
|
errorMessages.join("\n\n"),
|
||||||
|
error.stack,
|
||||||
|
elements.map((x) => stringifyElement(x)),
|
||||||
|
...additionalContext,
|
||||||
|
);
|
||||||
|
|
||||||
|
if (shouldThrow) {
|
||||||
|
// if enabled, gather all the errors first, throw once
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
@ -83,10 +149,15 @@ export const syncMovedIndices = (
|
|||||||
|
|
||||||
// try generatating indices, throws on invalid movedElements
|
// try generatating indices, throws on invalid movedElements
|
||||||
const elementsUpdates = generateIndices(elements, indicesGroups);
|
const elementsUpdates = generateIndices(elements, indicesGroups);
|
||||||
|
const elementsCandidates = elements.map((x) =>
|
||||||
|
elementsUpdates.has(x) ? { ...x, ...elementsUpdates.get(x) } : x,
|
||||||
|
);
|
||||||
|
|
||||||
// ensure next indices are valid before mutation, throws on invalid ones
|
// ensure next indices are valid before mutation, throws on invalid ones
|
||||||
validateFractionalIndices(
|
validateFractionalIndices(
|
||||||
elements.map((x) => elementsUpdates.get(x)?.index || x.index),
|
elementsCandidates,
|
||||||
|
// we don't autofix invalid bound text indices, hence don't include it in the validation
|
||||||
|
{ includeBoundTextValidation: false, shouldThrow: true },
|
||||||
);
|
);
|
||||||
|
|
||||||
// split mutation so we don't end up in an incosistent state
|
// split mutation so we don't end up in an incosistent state
|
||||||
|
1
packages/excalidraw/global.d.ts
vendored
1
packages/excalidraw/global.d.ts
vendored
@ -4,6 +4,7 @@ interface Window {
|
|||||||
EXCALIDRAW_ASSET_PATH: string | undefined;
|
EXCALIDRAW_ASSET_PATH: string | undefined;
|
||||||
EXCALIDRAW_EXPORT_SOURCE: string;
|
EXCALIDRAW_EXPORT_SOURCE: string;
|
||||||
EXCALIDRAW_THROTTLE_RENDER: boolean | undefined;
|
EXCALIDRAW_THROTTLE_RENDER: boolean | undefined;
|
||||||
|
DEBUG_FRACTIONAL_INDICES: boolean | undefined;
|
||||||
gtag: Function;
|
gtag: Function;
|
||||||
sa_event: Function;
|
sa_event: Function;
|
||||||
fathom: { trackEvent: Function };
|
fathom: { trackEvent: Function };
|
||||||
|
@ -274,9 +274,17 @@ class Scene {
|
|||||||
: Array.from(nextElements.values());
|
: Array.from(nextElements.values());
|
||||||
const nextFrameLikes: ExcalidrawFrameLikeElement[] = [];
|
const nextFrameLikes: ExcalidrawFrameLikeElement[] = [];
|
||||||
|
|
||||||
if (import.meta.env.DEV || import.meta.env.MODE === ENV.TEST) {
|
if (
|
||||||
// throw on invalid indices in test / dev to potentially detect cases were we forgot to sync moved elements
|
import.meta.env.DEV ||
|
||||||
validateFractionalIndices(_nextElements.map((x) => x.index));
|
import.meta.env.MODE === ENV.TEST ||
|
||||||
|
window?.DEBUG_FRACTIONAL_INDICES
|
||||||
|
) {
|
||||||
|
validateFractionalIndices(_nextElements, {
|
||||||
|
// validate everything
|
||||||
|
includeBoundTextValidation: true,
|
||||||
|
// throw only in dev & test, to remain functional on `DEBUG_FRACTIONAL_INDICES`
|
||||||
|
shouldThrow: import.meta.env.DEV || import.meta.env.MODE === ENV.TEST,
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
this.elements = syncInvalidIndices(_nextElements);
|
this.elements = syncInvalidIndices(_nextElements);
|
||||||
|
@ -763,7 +763,10 @@ function test(
|
|||||||
// ensure the input is invalid (unless the flag is on)
|
// ensure the input is invalid (unless the flag is on)
|
||||||
if (!expectValidInput) {
|
if (!expectValidInput) {
|
||||||
expect(() =>
|
expect(() =>
|
||||||
validateFractionalIndices(elements.map((x) => x.index)),
|
validateFractionalIndices(elements, {
|
||||||
|
shouldThrow: true,
|
||||||
|
includeBoundTextValidation: true,
|
||||||
|
}),
|
||||||
).toThrowError(InvalidFractionalIndexError);
|
).toThrowError(InvalidFractionalIndexError);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -777,7 +780,10 @@ function test(
|
|||||||
|
|
||||||
expect(syncedElements.length).toBe(elements.length);
|
expect(syncedElements.length).toBe(elements.length);
|
||||||
expect(() =>
|
expect(() =>
|
||||||
validateFractionalIndices(syncedElements.map((x) => x.index)),
|
validateFractionalIndices(syncedElements, {
|
||||||
|
shouldThrow: true,
|
||||||
|
includeBoundTextValidation: true,
|
||||||
|
}),
|
||||||
).not.toThrowError(InvalidFractionalIndexError);
|
).not.toThrowError(InvalidFractionalIndexError);
|
||||||
|
|
||||||
syncedElements.forEach((synced, index) => {
|
syncedElements.forEach((synced, index) => {
|
||||||
|
Loading…
x
Reference in New Issue
Block a user