From 11ba6784aa06119f3aaf2a7cca3a5ce1526dd720 Mon Sep 17 00:00:00 2001 From: Ryan Di Date: Mon, 23 Mar 2026 17:30:05 +1100 Subject: [PATCH] refactor: migrate elements by schema in restore --- packages/element/src/newElement.ts | 3 + packages/element/src/types.ts | 2 + packages/excalidraw/data/restore.ts | 58 ++++++------- packages/excalidraw/data/schema.ts | 124 ++++------------------------ packages/excalidraw/data/types.ts | 3 - 5 files changed, 44 insertions(+), 146 deletions(-) diff --git a/packages/element/src/newElement.ts b/packages/element/src/newElement.ts index ec50a81ff2..3e9f06d12e 100644 --- a/packages/element/src/newElement.ts +++ b/packages/element/src/newElement.ts @@ -50,6 +50,8 @@ import type { ExcalidrawLineElement, } from "./types"; +const ELEMENT_SCHEMA_VERSION = 2; + export type ElementConstructorOpts = MarkOptional< Omit, | "width" @@ -144,6 +146,7 @@ const _newElementBase = ( roundness, seed: rest.seed ?? randomInteger(), version: rest.version || 1, + schemaVersion: rest.schemaVersion ?? ELEMENT_SCHEMA_VERSION, versionNonce: rest.versionNonce ?? 0, isDeleted: false as false, boundElements, diff --git a/packages/element/src/types.ts b/packages/element/src/types.ts index 8067342a20..29ce6063d9 100644 --- a/packages/element/src/types.ts +++ b/packages/element/src/types.ts @@ -58,6 +58,8 @@ type _ExcalidrawElementBase = Readonly<{ /** Integer that is sequentially incremented on each change. Used to reconcile elements during collaboration or when saving to server. */ version: number; + /** Schema version hint used for per-element migration on restore. */ + schemaVersion?: number; /** Random integer that is regenerated on each change. Used for deterministic reconciliation of updates during collaboration, in case the versions (see above) are identical. */ diff --git a/packages/excalidraw/data/restore.ts b/packages/excalidraw/data/restore.ts index d7eb3d2216..8231642adb 100644 --- a/packages/excalidraw/data/restore.ts +++ b/packages/excalidraw/data/restore.ts @@ -82,11 +82,7 @@ import { getNormalizedZoom, } from "../scene"; -import { - migrateLibraryElements, - type SchemaVersionSource, - SCHEMA_VERSIONS, -} from "./schema"; +import { migrateElements, SCHEMA_VERSIONS } from "./schema"; import type { AppState, @@ -249,7 +245,7 @@ const repairBinding = ( }; const restoreElementWithProperties = < - T extends Required> & { + T extends Omit & { customData?: ExcalidrawElement["customData"]; /** @deprecated */ boundElementIds?: readonly ExcalidrawElement["id"][]; @@ -291,6 +287,7 @@ const restoreElementWithProperties = < width: element.width || 0, height: element.height || 0, seed: element.seed ?? 1, + schemaVersion: element.schemaVersion ?? SCHEMA_VERSIONS.latest, groupIds: element.groupIds ?? [], frameId: element.frameId ?? null, roundness: element.roundness @@ -645,14 +642,18 @@ export const restoreElements = ( } | undefined, ): CombineBrandsIfNeeded => { + const migratedTargetElements = migrateElements( + targetElements as readonly ExcalidrawElement[] | undefined | null, + ) as readonly T[] | undefined | null; + // used to detect duplicate top-level element ids const existingIds = new Set(); - const targetElementsMap = arrayToMap(targetElements || []); + const targetElementsMap = arrayToMap(migratedTargetElements || []); const existingElementsMap = existingElements ? arrayToMap(existingElements) : null; const restoredElements = syncInvalidIndices( - (targetElements || []).reduce((elements, element) => { + (migratedTargetElements || []).reduce((elements, element) => { // filtering out selection, which is legacy, no longer kept in elements, // and causing issues if retained if (element.type === "selection") { @@ -962,15 +963,9 @@ export const restoreAppState = ( }; }; -const restoreLibraryItem = ( - libraryItem: LibraryItem, - schemaVersionSource: SchemaVersionSource, -) => { +const restoreLibraryItem = (libraryItem: LibraryItem) => { const elements = restoreElements( - migrateLibraryElements( - getNonDeletedElements(libraryItem.elements), - schemaVersionSource, - ), + getNonDeletedElements(libraryItem.elements), null, ); return elements.length ? { ...libraryItem, elements } : null; @@ -979,21 +974,17 @@ const restoreLibraryItem = ( export const restoreLibraryItems = ( libraryItems: ImportedDataState["libraryItems"] = [], defaultStatus: LibraryItem["status"], - schemaVersionSource: SchemaVersionSource = SCHEMA_VERSIONS.latest, ) => { const restoredItems: LibraryItem[] = []; for (const item of libraryItems) { // migrate older libraries if (Array.isArray(item)) { - const restoredItem = restoreLibraryItem( - { - status: defaultStatus, - elements: item, - id: randomId(), - created: Date.now(), - }, - schemaVersionSource, - ); + const restoredItem = restoreLibraryItem({ + status: defaultStatus, + elements: item, + id: randomId(), + created: Date.now(), + }); if (restoredItem) { restoredItems.push(restoredItem); } @@ -1002,15 +993,12 @@ export const restoreLibraryItems = ( LibraryItem, "id" | "status" | "created" >; - const restoredItem = restoreLibraryItem( - { - ..._item, - id: _item.id || randomId(), - status: _item.status || defaultStatus, - created: _item.created || Date.now(), - }, - schemaVersionSource, - ); + const restoredItem = restoreLibraryItem({ + ..._item, + id: _item.id || randomId(), + status: _item.status || defaultStatus, + created: _item.created || Date.now(), + }); if (restoredItem) { restoredItems.push(restoredItem); } diff --git a/packages/excalidraw/data/schema.ts b/packages/excalidraw/data/schema.ts index 6c6dcf2dde..50ec2a3fd1 100644 --- a/packages/excalidraw/data/schema.ts +++ b/packages/excalidraw/data/schema.ts @@ -8,36 +8,21 @@ export const SCHEMA_VERSIONS = { latest: 2, } as const; -export const SCHEMA_MIGRATION_SCOPES = [ - "scene", - "library", - "clipboard", - "api", -] as const; - -export type SchemaMigrationScope = typeof SCHEMA_MIGRATION_SCOPES[number]; - -export const ALL_SCOPES: readonly SchemaMigrationScope[] = - SCHEMA_MIGRATION_SCOPES; - /** * Schema migration contract: * - `version`: integer schema version, strictly increasing. * - `title`: short human-readable label. * - `description`: required plain-language explanation of the change. - * - `scope`: one or more ingestion boundaries (`scene`, `library`, `clipboard`, `api`). * - `apply`: pure element transform. * * Rules: * - Use integer versions only. `SCHEMA_VERSIONS.latest` must match the last migration version. - * - Prefer explicit scopes; use `ALL_SCOPES` only when the same transform is required everywhere. * - Migrations should depend only on stable persisted schema fields (not temporary/PR-only fields). */ export type SchemaMigration = { version: number; title: string; description: string; - scope: readonly SchemaMigrationScope[]; apply: ( elements: readonly ExcalidrawElement[], ) => readonly ExcalidrawElement[]; @@ -49,7 +34,6 @@ export const SCHEMA_MIGRATIONS: readonly SchemaMigration[] = [ title: "Normalize legacy frame backgrounds", description: "Frames saved before schema v2 must render without visible fill, so normalize their backgroundColor to transparent on restore.", - scope: ALL_SCOPES, apply: (elements) => elements.map((element) => { if (element.type !== "frame") { @@ -63,37 +47,16 @@ export const SCHEMA_MIGRATIONS: readonly SchemaMigration[] = [ }, ]; -export const resolveSchemaVersion = ( - schemaVersion: number | undefined, - fallbackVersion: number, -) => { +export const resolveSchemaVersion = (schemaVersion: number | undefined) => { if ( Number.isInteger(schemaVersion) && (schemaVersion as number) >= SCHEMA_VERSIONS.initial ) { return schemaVersion as number; } - return fallbackVersion; + return SCHEMA_VERSIONS.initial; }; -const isValidSchemaVersion = ( - schemaVersion: number | undefined, -): schemaVersion is number => { - return ( - Number.isInteger(schemaVersion) && - (schemaVersion as number) >= SCHEMA_VERSIONS.initial - ); -}; - -export const hasElementSchemaVersion = ( - elements: readonly ExcalidrawElement[] | null | undefined, -) => - !!elements?.some((element) => - isValidSchemaVersion( - (element as ExcalidrawElement & { schemaVersion?: number }).schemaVersion, - ), - ); - export const validateSchemaMigrations = ( migrations: readonly SchemaMigration[], ) => { @@ -134,18 +97,6 @@ export const validateSchemaMigrations = ( `Migration "${migration.title}" must include a non-empty description.`, ); } - if (!migration.scope.length) { - errors.push( - `Migration "${migration.title}" must declare at least one scope.`, - ); - } - for (const scope of migration.scope) { - if (!SCHEMA_MIGRATION_SCOPES.includes(scope)) { - errors.push( - `Migration "${migration.title}" contains unsupported scope "${scope}".`, - ); - } - } } if (migrations.length > 0 && previousVersion !== SCHEMA_VERSIONS.latest) { @@ -171,7 +122,6 @@ export const migrateElementsBySchema = ( elements: readonly ExcalidrawElement[] | null | undefined, opts: { schemaVersion: number; - scope: SchemaMigrationScope; }, ) => { if (!elements) { @@ -183,74 +133,32 @@ export const migrateElementsBySchema = ( if (migration.version <= opts.schemaVersion) { return acc; } - if (!migration.scope.includes(opts.scope)) { - return acc; - } return migration.apply(acc); }, elements, ); }; -export type SchemaVersionSource = - | number - | { - payloadSchemaVersion?: number; - fallbackVersion: number; - }; - -const migrateElementsByScope = ( +export const migrateElements = ( elements: readonly ExcalidrawElement[] | null | undefined, - scope: SchemaMigrationScope, - source: SchemaVersionSource, ) => { if (!elements) { return elements; } - if (typeof source === "number") { - return migrateElementsBySchema(elements, { - schemaVersion: source, - scope, - }); - } - - if (isValidSchemaVersion(source.payloadSchemaVersion)) { - return migrateElementsBySchema(elements, { - schemaVersion: source.payloadSchemaVersion, - scope, - }); - } - return elements.map((element) => { - const schemaVersion = resolveSchemaVersion( - (element as ExcalidrawElement & { schemaVersion?: number }).schemaVersion, - source.fallbackVersion, - ); - const migratedElement = migrateElementsBySchema([element], { - schemaVersion, - scope, - }); - return migratedElement?.[0] || element; + const schemaVersion = resolveSchemaVersion(element.schemaVersion); + const migratedElement = + migrateElementsBySchema([element], { + schemaVersion, + })?.[0] || element; + + if ( + resolveSchemaVersion(migratedElement.schemaVersion) < + SCHEMA_VERSIONS.latest + ) { + (migratedElement as any).schemaVersion = SCHEMA_VERSIONS.latest; + } + return migratedElement; }); }; - -export const migrateSceneElements = ( - elements: readonly ExcalidrawElement[] | null | undefined, - source: SchemaVersionSource, -) => migrateElementsByScope(elements, "scene", source); - -export const migrateLibraryElements = ( - elements: readonly ExcalidrawElement[] | null | undefined, - source: SchemaVersionSource, -) => migrateElementsByScope(elements, "library", source); - -export const migrateClipboardElements = ( - elements: readonly ExcalidrawElement[] | null | undefined, - source: SchemaVersionSource, -) => migrateElementsByScope(elements, "clipboard", source); - -export const migrateAPIElements = ( - elements: readonly ExcalidrawElement[] | null | undefined, - source: SchemaVersionSource, -) => migrateElementsByScope(elements, "api", source); diff --git a/packages/excalidraw/data/types.ts b/packages/excalidraw/data/types.ts index 615a59b29d..94947c2b98 100644 --- a/packages/excalidraw/data/types.ts +++ b/packages/excalidraw/data/types.ts @@ -14,7 +14,6 @@ import type { export interface ExportedDataState { type: string; version: number; - schemaVersion: number; source: string; elements: readonly ExcalidrawElement[]; appState: ReturnType; @@ -36,7 +35,6 @@ export type LegacyAppState = { export interface ImportedDataState { type?: string; version?: number; - schemaVersion?: number; source?: string; elements?: readonly ExcalidrawElement[] | null; appState?: Readonly< @@ -54,7 +52,6 @@ export interface ImportedDataState { export interface ExportedLibraryData { type: string; version: typeof VERSIONS.excalidrawLibrary; - schemaVersion: number; source: string; libraryItems: LibraryItems; }