diff --git a/frontend/src/components/editor/renderers/__tests__/plugins.test.ts b/frontend/src/components/editor/renderers/__tests__/plugins.test.ts new file mode 100644 index 00000000000..9d0dd14d0a0 --- /dev/null +++ b/frontend/src/components/editor/renderers/__tests__/plugins.test.ts @@ -0,0 +1,90 @@ +/* Copyright 2026 Marimo. All rights reserved. */ + +import { describe, expect, it } from "vitest"; +import type { CellId } from "@/core/cells/ids"; +import type { CellData } from "@/core/cells/types"; +import { deserializeLayout, getCellRendererPlugin } from "../plugins"; + +function makeCell(id: string): CellData { + return { + id: id as CellId, + name: id, + code: "", + edited: false, + lastCodeRun: null, + lastExecutionTime: null, + config: { hide_code: false, disabled: false, column: null }, + serializedEditorState: null, + }; +} + +describe("getCellRendererPlugin", () => { + it("returns the matching plugin keyed by layout type", () => { + expect(getCellRendererPlugin("vertical").type).toBe("vertical"); + expect(getCellRendererPlugin("grid").type).toBe("grid"); + expect(getCellRendererPlugin("slides").type).toBe("slides"); + }); +}); + +describe("deserializeLayout", () => { + it("deserializes valid grid layout data", () => { + const layout = deserializeLayout({ + type: "grid", + data: { + columns: 12, + rowHeight: 20, + cells: [{ position: [1, 2, 3, 4] }], + }, + cells: [makeCell("a")], + }); + + expect(layout.columns).toBe(12); + expect(layout.cells).toEqual([{ i: "a", x: 1, y: 2, w: 3, h: 4 }]); + }); + + it("deserializes valid slides layout data", () => { + const layout = deserializeLayout({ + type: "slides", + data: { + cells: [{ type: "fragment" }], + deck: { transition: "fade" }, + }, + cells: [makeCell("a")], + }); + + expect(layout.deck).toEqual({ transition: "fade" }); + expect(layout.cells.get("a" as CellId)).toEqual({ type: "fragment" }); + }); + + it("vertical layout is always null regardless of stored data", () => { + // Older save files may have arbitrary `data` for vertical; we must + // ignore it because `VerticalLayout = null`. + const layout = deserializeLayout({ + type: "vertical", + data: { something: "unexpected" }, + cells: [makeCell("a")], + }); + + expect(layout).toBeNull(); + }); + + it("tolerates legacy `null` for optional grid fields", () => { + // Older marimo versions wrote unset optional fields as `null` + // (e.g. `"maxWidth": null` in `layout_grid_with_sidebar.grid.json`). + // Those files must keep loading. + const layout = deserializeLayout({ + type: "grid", + data: { + columns: 24, + rowHeight: 20, + maxWidth: null, + bordered: true, + cells: [{ position: [0, 0, 5, 2] }, { position: null }], + }, + cells: [makeCell("a"), makeCell("b")], + }); + + expect(layout.columns).toBe(24); + expect(layout.cells).toEqual([{ i: "a", x: 0, y: 0, w: 5, h: 2 }]); + }); +}); diff --git a/frontend/src/components/editor/renderers/cells-renderer.tsx b/frontend/src/components/editor/renderers/cells-renderer.tsx index 713f3a68e2f..904d344d03e 100644 --- a/frontend/src/components/editor/renderers/cells-renderer.tsx +++ b/frontend/src/components/editor/renderers/cells-renderer.tsx @@ -6,18 +6,11 @@ import { memo, type PropsWithChildren } from "react"; import { flattenTopLevelNotebookCells, useNotebook } from "@/core/cells/cells"; import type { AppConfig } from "@/core/config/config-schema"; import { KnownQueryParams } from "@/core/constants"; -import { - type LayoutData, - useLayoutActions, - useLayoutState, -} from "@/core/layout/layout"; +import { useLayoutActions, useLayoutState } from "@/core/layout/layout"; import { type AppMode, kioskModeAtom } from "@/core/mode"; -import { cellRendererPlugins } from "./plugins"; -import { - type ICellRendererPlugin, - type LayoutType, - OVERRIDABLE_LAYOUT_TYPES, -} from "./types"; +import { logNever } from "@/utils/assertNever"; +import { getCellRendererPlugin, type LayoutDataByType } from "./plugins"; +import { type LayoutType, OVERRIDABLE_LAYOUT_TYPES } from "./types"; interface Props { appConfig: AppConfig; @@ -46,20 +39,12 @@ export const CellsRenderer: React.FC> = memo( } } - const plugin = cellRendererPlugins.find((p) => p.type === finalLayout); - - // Just render children if there is no plugin - if (!plugin) { - return children; - } - return ( ); }, @@ -69,28 +54,42 @@ CellsRenderer.displayName = "CellsRenderer"; interface PluginCellRendererProps extends PropsWithChildren { appConfig: AppConfig; mode: AppMode; - // oxlint-disable-next-line typescript/no-explicit-any - plugin: ICellRendererPlugin; - layoutData: Partial>; + layoutData: Partial; finalLayout: LayoutType; } export const PluginCellRenderer = (props: PluginCellRendererProps) => { - const { appConfig, mode, plugin, layoutData, finalLayout } = props; + const { appConfig, mode, layoutData, finalLayout } = props; const notebook = useNotebook(); const { setCurrentLayoutData } = useLayoutActions(); const cells = flattenTopLevelNotebookCells(notebook); - const Renderer = plugin.Component; - const body = ( - - ); + const renderFor = ( + type: K, + data: LayoutDataByType[K] | undefined, + ) => { + const plugin = getCellRendererPlugin(type); + const Renderer = plugin.Component; + return ( + + ); + }; - return body; + switch (finalLayout) { + case "vertical": + return renderFor("vertical", layoutData.vertical); + case "grid": + return renderFor("grid", layoutData.grid); + case "slides": + return renderFor("slides", layoutData.slides); + default: + logNever(finalLayout); + return null; + } }; diff --git a/frontend/src/components/editor/renderers/plugins.ts b/frontend/src/components/editor/renderers/plugins.ts index 8532a29e1a4..39965d8f448 100644 --- a/frontend/src/components/editor/renderers/plugins.ts +++ b/frontend/src/components/editor/renderers/plugins.ts @@ -2,30 +2,60 @@ import type { CellData } from "@/core/cells/types"; import { GridLayoutPlugin } from "./grid-layout/plugin"; +import type { GridLayout, SerializedGridLayout } from "./grid-layout/types"; import { SlidesLayoutPlugin } from "./slides-layout/plugin"; +import type { + SerializedSlidesLayout, + SlidesLayout, +} from "./slides-layout/types"; import type { ICellRendererPlugin, LayoutType } from "./types"; +import type { + SerializedVerticalLayout, + VerticalLayout, +} from "./vertical-layout/types.ts"; import { VerticalLayoutPlugin } from "./vertical-layout/vertical-layout"; +export interface LayoutDataByType { + vertical: VerticalLayout; + grid: GridLayout; + slides: SlidesLayout; +} + +interface SerializedLayoutDataByType { + vertical: SerializedVerticalLayout; + grid: SerializedGridLayout; + slides: SerializedSlidesLayout; +} + +type CellRendererPluginByType = { + [K in LayoutType]: ICellRendererPlugin< + SerializedLayoutDataByType[K], + LayoutDataByType[K] + >; +}; + // If more renderers are added, we may want to consider lazy loading them. -// oxlint-disable-next-line typescript/no-explicit-any -export const cellRendererPlugins: ICellRendererPlugin[] = [ - GridLayoutPlugin, - SlidesLayoutPlugin, - VerticalLayoutPlugin, -]; +const cellRendererPluginMap: CellRendererPluginByType = { + vertical: VerticalLayoutPlugin, + grid: GridLayoutPlugin, + slides: SlidesLayoutPlugin, +}; + +export function getCellRendererPlugin( + type: K, +): CellRendererPluginByType[K] { + return cellRendererPluginMap[type]; +} -export function deserializeLayout({ +export function deserializeLayout({ type, data, cells, }: { - type: LayoutType; + type: K; data: unknown; cells: CellData[]; -}) { - const plugin = cellRendererPlugins.find((plugin) => plugin.type === type); - if (plugin === undefined) { - throw new Error(`Unknown layout type: ${type}`); - } - return plugin.deserializeLayout(data, cells); +}): LayoutDataByType[K] { + const plugin = getCellRendererPlugin(type); + return plugin.deserializeLayout(data as SerializedLayoutDataByType[K], cells); } diff --git a/frontend/src/components/editor/renderers/vertical-layout/types.ts b/frontend/src/components/editor/renderers/vertical-layout/types.ts new file mode 100644 index 00000000000..eacf49ec5a6 --- /dev/null +++ b/frontend/src/components/editor/renderers/vertical-layout/types.ts @@ -0,0 +1,5 @@ +/* Copyright 2026 Marimo. All rights reserved. */ + +export type SerializedVerticalLayout = unknown; + +export type VerticalLayout = null; diff --git a/frontend/src/components/editor/renderers/vertical-layout/vertical-layout.tsx b/frontend/src/components/editor/renderers/vertical-layout/vertical-layout.tsx index 10e380ad882..0504f537bb2 100644 --- a/frontend/src/components/editor/renderers/vertical-layout/vertical-layout.tsx +++ b/frontend/src/components/editor/renderers/vertical-layout/vertical-layout.tsx @@ -52,10 +52,9 @@ import { Filenames } from "@/utils/filenames"; import { FloatingOutline } from "../../chrome/panels/outline/floating-outline"; import { cellDomProps } from "../../common"; import type { ICellRendererPlugin, ICellRendererProps } from "../types"; +import type { SerializedVerticalLayout, VerticalLayout } from "./types.ts"; import { useDelayVisibility } from "./useDelayVisibility"; import { VerticalLayoutWrapper } from "./vertical-layout-wrapper"; - -type VerticalLayout = null; type VerticalLayoutProps = ICellRendererProps; const VerticalLayoutRenderer: React.FC = ({ @@ -468,15 +467,15 @@ const VerticalCell = memo( VerticalCell.displayName = "VerticalCell"; export const VerticalLayoutPlugin: ICellRendererPlugin< - VerticalLayout, + SerializedVerticalLayout, VerticalLayout > = { type: "vertical", name: "Vertical", - validator: z.any(), + validator: z.unknown(), Component: VerticalLayoutRenderer, - deserializeLayout: (serialized) => serialized, - serializeLayout: (layout) => layout, + deserializeLayout: () => null, + serializeLayout: () => null, getInitialLayout: () => null, }; diff --git a/frontend/src/core/kernel/__tests__/handlers.test.ts b/frontend/src/core/kernel/__tests__/handlers.test.ts index 7048cfa2dab..89eec4fc100 100644 --- a/frontend/src/core/kernel/__tests__/handlers.test.ts +++ b/frontend/src/core/kernel/__tests__/handlers.test.ts @@ -1,10 +1,43 @@ /* Copyright 2026 Marimo. All rights reserved. */ import { describe, expect, it, vi } from "vitest"; import { cellId } from "@/__tests__/branded"; +import { Logger } from "@/utils/Logger"; import { buildCellData, buildLayoutState } from "../handlers"; import type { NotificationMessageData } from "../messages"; import { queryParamHandlers } from "../queryParamHandlers"; +type KernelReadyMessage = NotificationMessageData<"kernel-ready">; + +// Builds a minimal kernel-ready message with a one-cell notebook so the +// layout-state tests can focus on the `layout` field. +function makeKernelReady( + layout: KernelReadyMessage["layout"], +): KernelReadyMessage { + return { + cell_ids: [cellId("cell1")], + codes: ["x = 1"], + names: ["__"], + configs: [{ disabled: false, hide_code: false }], + layout, + resumed: false, + ui_values: {}, + last_executed_code: {}, + last_execution_time: {}, + app_config: { + width: "normal", + app_title: null, + layout_file: null, + css_file: null, + auto_download: [], + }, + kiosk: false, + capabilities: { + terminal: false, + }, + auto_instantiated: false, + }; +} + // Helper to set up URL and searchParams function setupURL(search = "") { const url = new URL("http://localhost:3000"); @@ -227,11 +260,11 @@ describe("buildLayoutState", () => { const cells = buildCellData(kernelReadyData); const mockSetLayoutData = vi.fn(); - const layoutState = buildLayoutState( - kernelReadyData, + const layoutState = buildLayoutState({ + data: kernelReadyData, cells, - mockSetLayoutData, - ); + setLayoutData: mockSetLayoutData, + }); expect(layoutState).toMatchInlineSnapshot(` { @@ -275,16 +308,100 @@ describe("buildLayoutState", () => { const cells = buildCellData(kernelReadyData); const mockSetLayoutData = vi.fn(); - const layoutState = buildLayoutState( - kernelReadyData, + const layoutState = buildLayoutState({ + data: kernelReadyData, cells, - mockSetLayoutData, - ); + setLayoutData: mockSetLayoutData, + }); expect(layoutState.selectedLayout).toBe("vertical"); expect(mockSetLayoutData).toHaveBeenCalledWith({ layoutView: "vertical", - data: expect.any(Array), + data: null, + }); + }); + + it("should build layout state with grid layout", () => { + const kernelReadyData = makeKernelReady({ + type: "grid", + data: { + columns: 12, + rowHeight: 20, + cells: [{ position: [0, 0, 4, 3] }], + }, + }); + + const cells = buildCellData(kernelReadyData); + const mockSetLayoutData = vi.fn(); + const layoutState = buildLayoutState({ + data: kernelReadyData, + cells, + setLayoutData: mockSetLayoutData, + }); + + expect(layoutState.selectedLayout).toBe("grid"); + expect(layoutState.layoutData.grid).toBeDefined(); + expect(layoutState.layoutData.grid?.cells).toEqual([ + { i: "cell1", x: 0, y: 0, w: 4, h: 3 }, + ]); + expect(mockSetLayoutData).toHaveBeenCalledTimes(1); + expect(mockSetLayoutData).toHaveBeenCalledWith({ + layoutView: "grid", + data: layoutState.layoutData.grid, + }); + }); + + it("should build layout state with slides layout", () => { + const kernelReadyData = makeKernelReady({ + type: "slides", + data: { + cells: [{ type: "slide" }], + deck: { transition: "fade" }, + }, + }); + + const cells = buildCellData(kernelReadyData); + const mockSetLayoutData = vi.fn(); + const layoutState = buildLayoutState({ + data: kernelReadyData, + cells, + setLayoutData: mockSetLayoutData, + }); + + expect(layoutState.selectedLayout).toBe("slides"); + expect(layoutState.layoutData.slides?.deck).toEqual({ transition: "fade" }); + expect(layoutState.layoutData.slides?.cells.get(cellId("cell1"))).toEqual({ + type: "slide", + }); + expect(mockSetLayoutData).toHaveBeenCalledTimes(1); + expect(mockSetLayoutData).toHaveBeenCalledWith({ + layoutView: "slides", + data: layoutState.layoutData.slides, + }); + }); + + it("should ignore unknown layout types and warn", () => { + const warnSpy = vi.spyOn(Logger, "warn").mockImplementation(() => {}); + const kernelReadyData = makeKernelReady({ + // oxlint-disable-next-line typescript/no-explicit-any + type: "totally-bogus" as any, + data: {}, }); + + const cells = buildCellData(kernelReadyData); + const mockSetLayoutData = vi.fn(); + const layoutState = buildLayoutState({ + data: kernelReadyData, + cells, + setLayoutData: mockSetLayoutData, + }); + + expect(layoutState.selectedLayout).toBe("vertical"); + expect(layoutState.layoutData).toEqual({}); + expect(mockSetLayoutData).not.toHaveBeenCalled(); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('unknown layout type "totally-bogus"'), + ); + warnSpy.mockRestore(); }); }); diff --git a/frontend/src/core/kernel/handlers.ts b/frontend/src/core/kernel/handlers.ts index 04bc225df5c..6212b089dc7 100644 --- a/frontend/src/core/kernel/handlers.ts +++ b/frontend/src/core/kernel/handlers.ts @@ -1,6 +1,13 @@ /* Copyright 2026 Marimo. All rights reserved. */ -import { deserializeLayout } from "@/components/editor/renderers/plugins"; -import type { LayoutType } from "@/components/editor/renderers/types"; +import { + deserializeLayout, + type LayoutDataByType, +} from "@/components/editor/renderers/plugins"; +import { + LAYOUT_TYPES, + type LayoutType, +} from "@/components/editor/renderers/types"; +import { logNever } from "@/utils/assertNever"; import { Logger } from "@/utils/Logger"; import { Objects } from "@/utils/objects"; import type { UIElementId } from "../cells/ids"; @@ -9,8 +16,8 @@ import { type AppConfig, AppConfigSchema } from "../config/config-schema"; import { UI_ELEMENT_REGISTRY } from "../dom/uiregistry"; import { initialLayoutState, - type LayoutData, type LayoutState, + type SetLayoutDataPayload, } from "../layout/layout"; import { getRequestClient } from "../network/requests"; import { VirtualFileTracker } from "../static/virtual-file-tracker"; @@ -61,29 +68,50 @@ export function buildCellData(data: KernelReadyData): CellData[] { /** * Build layout state from kernel-ready data. */ -export function buildLayoutState( - data: KernelReadyData, - cells: CellData[], - setLayoutData: (payload: { - layoutView: LayoutType; - data: LayoutData; - }) => void, -): LayoutState { +export function buildLayoutState(opts: { + data: KernelReadyData; + cells: CellData[]; + setLayoutData: (payload: SetLayoutDataPayload) => void; +}): LayoutState { + const { data, cells, setLayoutData } = opts; const layoutState = initialLayoutState(); const { layout } = data; - if (layout) { - const layoutType = layout.type as LayoutType; - const layoutData = deserializeLayout({ - type: layoutType, - data: layout.data, - cells, - }); - layoutState.selectedLayout = layoutType; - layoutState.layoutData[layoutType] = layoutData; - setLayoutData({ layoutView: layoutType, data: layoutData }); + if (!layout) { + return layoutState; } + const layoutType = layout.type as LayoutType; + if (!LAYOUT_TYPES.includes(layoutType)) { + Logger.warn( + `Ignoring unknown layout type "${layout.type}"; falling back to default.`, + ); + return layoutState; + } + + const apply = ( + type: K, + onApply: (payload: { layoutView: K; data: LayoutDataByType[K] }) => void, + ) => { + const layoutData = deserializeLayout({ type, data: layout.data, cells }); + layoutState.selectedLayout = type; + layoutState.layoutData[type] = layoutData; + onApply({ layoutView: type, data: layoutData }); + }; + + switch (layoutType) { + case "vertical": + apply("vertical", setLayoutData); + break; + case "grid": + apply("grid", setLayoutData); + break; + case "slides": + apply("slides", setLayoutData); + break; + default: + logNever(layoutType); + } return layoutState; } @@ -105,10 +133,7 @@ export function handleKernelReady( opts: { autoInstantiate: boolean; setCells: (cells: CellData[], layout: LayoutState) => void; - setLayoutData: (payload: { - layoutView: LayoutType; - data: LayoutData; - }) => void; + setLayoutData: (payload: SetLayoutDataPayload) => void; setCapabilities: (capabilities: Capabilities) => void; setKernelState: (state: KernelState) => void; setAppConfig: (config: AppConfig) => void; @@ -140,7 +165,7 @@ export function handleKernelReady( hasExistingCells && !resumed ? existingCells : buildCellData(data); // Set up layout and cells - const layoutState = buildLayoutState(data, cells, setLayoutData); + const layoutState = buildLayoutState({ data, cells, setLayoutData }); setCells(cells, layoutState); // Set app config and capabilities diff --git a/frontend/src/core/layout/layout.ts b/frontend/src/core/layout/layout.ts index d885bb77666..bc8c59459c8 100644 --- a/frontend/src/core/layout/layout.ts +++ b/frontend/src/core/layout/layout.ts @@ -1,20 +1,25 @@ /* Copyright 2026 Marimo. All rights reserved. */ import { useAtomValue } from "jotai"; -import type { GridLayout } from "@/components/editor/renderers/grid-layout/types"; -import { cellRendererPlugins } from "@/components/editor/renderers/plugins"; +import { + getCellRendererPlugin, + type LayoutDataByType, +} from "@/components/editor/renderers/plugins"; import type { LayoutType } from "@/components/editor/renderers/types"; +import { logNever } from "@/utils/assertNever"; import { createReducerAndAtoms } from "@/utils/createReducer"; -import { Logger } from "@/utils/Logger"; import { getNotebook } from "../cells/cells"; import { notebookCells } from "../cells/utils"; import { store } from "../state/jotai"; -export type LayoutData = GridLayout | undefined; +export type LayoutData = LayoutDataByType[LayoutType]; +export type SetLayoutDataPayload = { + [K in LayoutType]: { layoutView: K; data: LayoutDataByType[K] }; +}[LayoutType]; export interface LayoutState { selectedLayout: LayoutType; - layoutData: Partial>; + layoutData: Partial; } export function initialLayoutState(): LayoutState { @@ -33,10 +38,7 @@ const { valueAtom: layoutStateAtom, useActions } = createReducerAndAtoms( selectedLayout: payload, }; }, - setLayoutData: ( - state, - payload: { layoutView: LayoutType; data: LayoutData }, - ) => { + setLayoutData: (state, payload: SetLayoutDataPayload) => { return { ...state, selectedLayout: payload.layoutView, @@ -79,25 +81,24 @@ export function getSerializedLayout() { if (selectedLayout === "vertical") { return null; } - - if (layoutData === undefined) { - return null; - } - - const plugin = cellRendererPlugins.find( - (plugin) => plugin.type === selectedLayout, - ); - if (plugin === undefined) { - Logger.error(`Unknown layout type: ${selectedLayout}`); - return null; - } const cells = notebookCells(notebook); + // Fall back to the plugin's initial layout when the user has not yet // interacted with this layout — otherwise serializers that expect a // structured layout object crash on `undefined`. - const data = layoutData[selectedLayout] ?? plugin.getInitialLayout(cells); - return { - type: selectedLayout, - data: plugin.serializeLayout(data, cells), + const serialize = (type: K) => { + const plugin = getCellRendererPlugin(type); + const data = layoutData[type] ?? plugin.getInitialLayout(cells); + return { type, data: plugin.serializeLayout(data, cells) }; }; + + switch (selectedLayout) { + case "grid": + return serialize("grid"); + case "slides": + return serialize("slides"); + default: + logNever(selectedLayout); + return null; + } }