fix: throttle fractional indices validation (#8306)

This commit is contained in:
Marcel Mraz 2024-08-02 11:55:15 +02:00 committed by GitHub
parent e63dd025c9
commit 84d89b9a8a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 71 additions and 40 deletions

View file

@ -1,3 +1,4 @@
import throttle from "lodash.throttle";
import { ENV } from "../constants"; import { ENV } from "../constants";
import type { OrderedExcalidrawElement } from "../element/types"; import type { OrderedExcalidrawElement } from "../element/types";
import { import {
@ -38,6 +39,37 @@ const shouldDiscardRemoteElement = (
return false; return false;
}; };
const validateIndicesThrottled = throttle(
(
orderedElements: readonly OrderedExcalidrawElement[],
localElements: readonly OrderedExcalidrawElement[],
remoteElements: readonly RemoteExcalidrawElement[],
) => {
if (
import.meta.env.DEV ||
import.meta.env.MODE === ENV.TEST ||
window?.DEBUG_FRACTIONAL_INDICES
) {
// create new instances due to the mutation
const elements = syncInvalidIndices(
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,
},
});
}
},
1000 * 60,
{ leading: true, trailing: false },
);
export const reconcileElements = ( export const reconcileElements = (
localElements: readonly OrderedExcalidrawElement[], localElements: readonly OrderedExcalidrawElement[],
remoteElements: readonly RemoteExcalidrawElement[], remoteElements: readonly RemoteExcalidrawElement[],
@ -77,26 +109,7 @@ export const reconcileElements = (
const orderedElements = orderByFractionalIndex(reconciledElements); const orderedElements = orderByFractionalIndex(reconciledElements);
if ( validateIndicesThrottled(orderedElements, localElements, remoteElements);
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);

View file

@ -37,10 +37,12 @@ export const validateFractionalIndices = (
{ {
shouldThrow = false, shouldThrow = false,
includeBoundTextValidation = false, includeBoundTextValidation = false,
ignoreLogs,
reconciliationContext, reconciliationContext,
}: { }: {
shouldThrow: boolean; shouldThrow: boolean;
includeBoundTextValidation: boolean; includeBoundTextValidation: boolean;
ignoreLogs?: true;
reconciliationContext?: { reconciliationContext?: {
localElements: ReadonlyArray<ExcalidrawElement>; localElements: ReadonlyArray<ExcalidrawElement>;
remoteElements: ReadonlyArray<ExcalidrawElement>; remoteElements: ReadonlyArray<ExcalidrawElement>;
@ -95,6 +97,7 @@ export const validateFractionalIndices = (
); );
} }
if (!ignoreLogs) {
// report just once and with the stacktrace // report just once and with the stacktrace
console.error( console.error(
errorMessages.join("\n\n"), errorMessages.join("\n\n"),
@ -102,6 +105,7 @@ export const validateFractionalIndices = (
elements.map((x) => stringifyElement(x)), elements.map((x) => stringifyElement(x)),
...additionalContext, ...additionalContext,
); );
}
if (shouldThrow) { if (shouldThrow) {
// if enabled, gather all the errors first, throw once // if enabled, gather all the errors first, throw once
@ -157,7 +161,11 @@ export const syncMovedIndices = (
validateFractionalIndices( validateFractionalIndices(
elementsCandidates, elementsCandidates,
// we don't autofix invalid bound text indices, hence don't include it in the validation // we don't autofix invalid bound text indices, hence don't include it in the validation
{ includeBoundTextValidation: false, shouldThrow: true }, {
includeBoundTextValidation: false,
shouldThrow: true,
ignoreLogs: 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

View file

@ -1,3 +1,4 @@
import throttle from "lodash.throttle";
import type { import type {
ExcalidrawElement, ExcalidrawElement,
NonDeletedExcalidrawElement, NonDeletedExcalidrawElement,
@ -50,6 +51,24 @@ const getNonDeletedElements = <T extends ExcalidrawElement>(
return { elementsMap, elements }; return { elementsMap, elements };
}; };
const validateIndicesThrottled = throttle(
(elements: readonly ExcalidrawElement[]) => {
if (
import.meta.env.DEV ||
import.meta.env.MODE === ENV.TEST ||
window?.DEBUG_FRACTIONAL_INDICES
) {
validateFractionalIndices(elements, {
// throw only in dev & test, to remain functional on `DEBUG_FRACTIONAL_INDICES`
shouldThrow: import.meta.env.DEV || import.meta.env.MODE === ENV.TEST,
includeBoundTextValidation: true,
});
}
},
1000 * 60,
{ leading: true, trailing: false },
);
const hashSelectionOpts = ( const hashSelectionOpts = (
opts: Parameters<InstanceType<typeof Scene>["getSelectedElements"]>[0], opts: Parameters<InstanceType<typeof Scene>["getSelectedElements"]>[0],
) => { ) => {
@ -274,18 +293,7 @@ class Scene {
: Array.from(nextElements.values()); : Array.from(nextElements.values());
const nextFrameLikes: ExcalidrawFrameLikeElement[] = []; const nextFrameLikes: ExcalidrawFrameLikeElement[] = [];
if ( validateIndicesThrottled(_nextElements);
import.meta.env.DEV ||
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);
this.elementsMap.clear(); this.elementsMap.clear();

View file

@ -766,6 +766,7 @@ function test(
validateFractionalIndices(elements, { validateFractionalIndices(elements, {
shouldThrow: true, shouldThrow: true,
includeBoundTextValidation: true, includeBoundTextValidation: true,
ignoreLogs: true,
}), }),
).toThrowError(InvalidFractionalIndexError); ).toThrowError(InvalidFractionalIndexError);
} }
@ -783,6 +784,7 @@ function test(
validateFractionalIndices(syncedElements, { validateFractionalIndices(syncedElements, {
shouldThrow: true, shouldThrow: true,
includeBoundTextValidation: true, includeBoundTextValidation: true,
ignoreLogs: true,
}), }),
).not.toThrowError(InvalidFractionalIndexError); ).not.toThrowError(InvalidFractionalIndexError);