Skip to content

Commit 1525d7a

Browse files
authored
improvement: fallback to available storage when localStorage is not available (#6879)
When iframing marimo in a sandboxed iframe, depending on configuration `localStorage` may not be available so this adds some appropriate fallbacks this also adds a lint rule (grit) to make sure we pass the optional storage into `atomWithStorage`
1 parent 747f83a commit 1525d7a

File tree

23 files changed

+220
-61
lines changed

23 files changed

+220
-61
lines changed

biome.jsonc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
"plugins": [
44
"frontend/lint/addEventListenerObject.grit",
55
"frontend/lint/removeEventListenerObject.grit",
6-
"frontend/lint/preferObjectParams.grit"
6+
"frontend/lint/preferObjectParams.grit",
7+
"frontend/lint/atomWithStorageArgs.grit"
78
],
89
"files": {
910
"includes": ["frontend/**/*", "packages/**/*", "!*.json", "!*.jsonc"]
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Enforce atomWithStorage to use at least 3 arguments
2+
// This ensures the storage parameter is always explicitly provided
3+
or {
4+
`atomWithStorage($arg1, $arg2)`,
5+
`atomWithStorage<$_>($arg1, $arg2)`
6+
} where {
7+
register_diagnostic(
8+
span=$arg1,
9+
message="atomWithStorage requires at least 3 arguments (key, defaultValue, storage). Provide the storage parameter explicitly.",
10+
severity="error"
11+
)
12+
}

frontend/src/components/chat/acp/state.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { atom } from "jotai";
44
import { atomWithStorage } from "jotai/utils";
55
import { capitalize } from "lodash-es";
66
import { isPlatformWindows } from "@/core/hotkeys/shortcuts";
7+
import { jotaiJsonStorage } from "@/utils/storage/jotai";
78
import type { TypedString } from "@/utils/typed";
89
import { generateUUID } from "@/utils/uuid";
910
import type { ExternalAgentSessionId, SessionSupportType } from "./types";
@@ -41,6 +42,7 @@ export const agentSessionStateAtom = atomWithStorage<AgentSessionState>(
4142
sessions: [],
4243
activeTabId: null,
4344
},
45+
jotaiJsonStorage,
4446
);
4547

4648
export const selectedTabAtom = atom(

frontend/src/components/data-table/charts/__tests__/storage.test.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,18 @@
22

33
import { getDefaultStore } from "jotai";
44
import { beforeEach, describe, expect, it, vi } from "vitest";
5-
import { asMock, SetupMocks } from "@/__mocks__/common";
5+
6+
// Mock the storage module to use our mock storage
7+
vi.mock("@/utils/storage/storage", () => ({
8+
availableStorage: {
9+
getItem: vi.fn(),
10+
setItem: vi.fn(),
11+
removeItem: vi.fn(),
12+
},
13+
}));
14+
615
import type { CellId } from "@/core/cells/ids";
16+
import { availableStorage } from "@/utils/storage/storage";
717
import { ChartSchema } from "../schemas";
818
import type { TabName } from "../storage";
919
import { KEY, tabsStorageAtom } from "../storage";
@@ -12,7 +22,6 @@ import { ChartType } from "../types";
1222
describe("Chart Transforms Storage", () => {
1323
beforeEach(() => {
1424
vi.clearAllMocks();
15-
SetupMocks.localStorage();
1625
// Reset the store before each test
1726
const store = getDefaultStore();
1827
store.set(tabsStorageAtom, new Map());
@@ -87,14 +96,11 @@ describe("Chart Transforms Storage", () => {
8796

8897
describe("LocalStorage integration", () => {
8998
it("should call localStorage.setItem when setting data", () => {
90-
// Reset the mock to ensure clean state
91-
asMock(window.localStorage.setItem).mockReset();
92-
9399
const store = getDefaultStore();
94100
const newMap = new Map();
95101
store.set(tabsStorageAtom, newMap);
96102

97-
expect(window.localStorage.setItem).toHaveBeenCalledWith(
103+
expect(availableStorage.setItem).toHaveBeenCalledWith(
98104
KEY,
99105
JSON.stringify([]),
100106
);

frontend/src/components/data-table/charts/storage.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { capitalize } from "lodash-es";
55
import { z } from "zod";
66
import type { CellId } from "@/core/cells/ids";
77
import { Logger } from "@/utils/Logger";
8-
import { NotebookScopedLocalStorage } from "@/utils/localStorage";
8+
import { NotebookScopedLocalStorage } from "@/utils/storage/typed";
99
import type { TypedString } from "@/utils/typed";
1010
import { ChartSchema, type ChartSchemaType } from "./schemas";
1111
import type { ChartType } from "./types";

frontend/src/components/editor/ai/add-cell-with-ai.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ import { useRuntimeManager } from "@/core/runtime/config";
5353
import { useTheme } from "@/theme/useTheme";
5454
import { cn } from "@/utils/cn";
5555
import { prettyError } from "@/utils/errors";
56-
import { ZodLocalStorage } from "@/utils/localStorage";
56+
import { jotaiJsonStorage } from "@/utils/storage/jotai";
57+
import { ZodLocalStorage } from "@/utils/storage/typed";
5758
import { PythonIcon } from "../cell/code/icons";
5859
import {
5960
CompletionActions,
@@ -66,6 +67,7 @@ import { StreamingChunkTransport } from "./transport/chat-transport";
6667
const languageAtom = atomWithStorage<"python" | "sql">(
6768
"marimo:ai-language",
6869
"python",
70+
jotaiJsonStorage,
6971
);
7072

7173
const KEY = "marimo:ai-prompt-history";

frontend/src/components/editor/chrome/state.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import { useAtomValue } from "jotai";
44
import { z } from "zod";
55
import { createReducerAndAtoms } from "@/utils/createReducer";
6-
import { ZodLocalStorage } from "@/utils/localStorage";
6+
import { ZodLocalStorage } from "@/utils/storage/typed";
77
import type { PanelType } from "./types";
88

99
export interface ChromeState {

frontend/src/components/editor/columns/storage.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { z } from "zod";
44
import { arrayMove } from "@/utils/arrays";
5-
import { NotebookScopedLocalStorage } from "@/utils/localStorage";
5+
import { NotebookScopedLocalStorage } from "@/utils/storage/typed";
66

77
const BASE_KEY = "marimo:notebook-col-sizes";
88

frontend/src/components/editor/errors/fix-mode.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,17 @@
22

33
import { useAtom } from "jotai";
44
import { atomWithStorage } from "jotai/utils";
5+
import { jotaiJsonStorage } from "@/utils/storage/jotai";
56

67
export type FixMode = "prompt" | "autofix";
78

89
const BASE_KEY = "marimo:ai-autofix-mode";
910

10-
const fixModeAtom = atomWithStorage<FixMode>(BASE_KEY, "autofix");
11+
const fixModeAtom = atomWithStorage<FixMode>(
12+
BASE_KEY,
13+
"autofix",
14+
jotaiJsonStorage,
15+
);
1116

1217
export function useFixMode() {
1318
const [fixMode, setFixMode] = useAtom(fixModeAtom);

frontend/src/components/editor/output/useWrapText.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,16 @@
22

33
import { useAtom } from "jotai";
44
import { atomWithStorage } from "jotai/utils";
5+
import { jotaiJsonStorage } from "@/utils/storage/jotai";
56

67
const WRAP_TEXT_KEY = "marimo:console:wrapText";
78

89
// Atom for storing wrap text preference - shared across all console outputs
9-
const wrapTextAtom = atomWithStorage<boolean>(WRAP_TEXT_KEY, false);
10+
const wrapTextAtom = atomWithStorage<boolean>(
11+
WRAP_TEXT_KEY,
12+
false,
13+
jotaiJsonStorage,
14+
);
1015

1116
export function useWrapText() {
1217
const [wrapText, setWrapText] = useAtom(wrapTextAtom);

0 commit comments

Comments
 (0)