Skip to content

Commit c559090

Browse files
authored
fix add setup cell overwriting original cell on refresh (#7061)
## 📝 Summary <!-- Provide a concise summary of what this pull request is addressing. If this PR fixes any issues, list them here by number (e.g., Fixes #123). --> Fixes #7052 . Adds new logic to detect whether a setup cell exists. ## 🔍 Description of Changes <!-- Detail the specific changes made in this pull request. Explain the problem addressed and how it was resolved. If applicable, provide before and after comparisons, screenshots, or any relevant details to help reviewers understand the changes easily. --> ## 📋 Checklist - [x] I have read the [contributor guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md). - [ ] For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on [Discord](https://marimo.io/discord?ref=pr), or the community [discussions](https://github.com/marimo-team/marimo/discussions) (Please provide a link if applicable). - [x] I have added tests for the changes made. - [x] I have run the code and verified that it works as expected.
1 parent 2625020 commit c559090

File tree

12 files changed

+36
-39
lines changed

12 files changed

+36
-39
lines changed

frontend/src/components/editor/actions/useCellActionButton.tsx

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,8 @@ import { Switch } from "@/components/ui/switch";
3939
import { toast } from "@/components/ui/use-toast";
4040
import { aiCompletionCellAtom } from "@/core/ai/state";
4141
import { maybeAddMarimoImport } from "@/core/cells/add-missing-import";
42-
import {
43-
hasOnlyOneCellAtom,
44-
SETUP_CELL_ID,
45-
useCellActions,
46-
} from "@/core/cells/cells";
47-
import type { CellId } from "@/core/cells/ids";
42+
import { hasOnlyOneCellAtom, useCellActions } from "@/core/cells/cells";
43+
import { type CellId, SETUP_CELL_ID } from "@/core/cells/ids";
4844
import type { CellData } from "@/core/cells/types";
4945
import { formatEditorViews } from "@/core/codemirror/format";
5046
import { toggleToLanguage } from "@/core/codemirror/language/commands";

frontend/src/components/editor/links/cell-link.tsx

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,8 @@
11
/* Copyright 2024 Marimo. All rights reserved. */
22

33
import type { JSX } from "react"; /* Copyright 2024 Marimo. All rights reserved. */
4-
import {
5-
SCRATCH_CELL_ID,
6-
useCellActions,
7-
useCellIds,
8-
useCellNames,
9-
} from "@/core/cells/cells";
10-
import { type CellId, HTMLCellId } from "@/core/cells/ids";
4+
import { useCellActions, useCellIds, useCellNames } from "@/core/cells/cells";
5+
import { type CellId, HTMLCellId, SCRATCH_CELL_ID } from "@/core/cells/ids";
116
import { displayCellName } from "@/core/cells/names";
127
import { goToCellLine } from "@/core/codemirror/go-to-definition/utils";
138
import { useFilename } from "@/core/saving/filename";

frontend/src/components/editor/notebook-cell.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,12 @@ import type { Milliseconds, Seconds } from "@/utils/time";
4141
import {
4242
type CellActions,
4343
createUntouchedCellAtom,
44-
SETUP_CELL_ID,
4544
useCellActions,
4645
useCellData,
4746
useCellHandle,
4847
useCellRuntime,
4948
} from "../../core/cells/cells";
50-
import type { CellId } from "../../core/cells/ids";
49+
import { type CellId, SETUP_CELL_ID } from "../../core/cells/ids";
5150
import { isUninstantiated } from "../../core/cells/utils";
5251
import type { UserConfig } from "../../core/config/config-schema";
5352
import {

frontend/src/components/editor/renderers/cell-array.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { SortableCellsProvider } from "@/components/sort/SortableCellsProvider";
2020
import { Button } from "@/components/ui/button";
2121
import { Tooltip } from "@/components/ui/tooltip";
2222
import { maybeAddMarimoImport } from "@/core/cells/add-missing-import";
23+
import { SETUP_CELL_ID } from "@/core/cells/ids";
2324
import { LanguageAdapters } from "@/core/codemirror/language/LanguageAdapters";
2425
import { aiEnabledAtom } from "@/core/config/config";
2526
import { isConnectedAtom } from "@/core/network/connection";
@@ -30,7 +31,6 @@ import type { CellColumnId } from "@/utils/id-tree";
3031
import { invariant } from "@/utils/invariant";
3132
import {
3233
columnIdsAtom,
33-
SETUP_CELL_ID,
3434
useCellActions,
3535
useCellIds,
3636
useScrollKey,

frontend/src/components/scratchpad/scratchpad.tsx

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,9 @@ import {
1111
import type React from "react";
1212
import { Suspense, useRef, useState } from "react";
1313
import useEvent from "react-use-event-hook";
14-
import {
15-
SCRATCH_CELL_ID,
16-
useCellActions,
17-
useNotebook,
18-
} from "@/core/cells/cells";
14+
import { useCellActions, useNotebook } from "@/core/cells/cells";
1915
import { useLastFocusedCellId } from "@/core/cells/focus";
20-
import { HTMLCellId } from "@/core/cells/ids";
16+
import { HTMLCellId, SCRATCH_CELL_ID } from "@/core/cells/ids";
2117
import { DEFAULT_CELL_NAME } from "@/core/cells/names";
2218
import type { LanguageAdapterType } from "@/core/codemirror/language/types";
2319
import { useResolvedMarimoConfig } from "@/core/config/config";

frontend/src/core/cells/__tests__/cells.test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
vi,
1414
} from "vitest";
1515
import type { CellHandle } from "@/components/editor/notebook-cell";
16-
import { CellId } from "@/core/cells/ids";
16+
import { CellId, SETUP_CELL_ID } from "@/core/cells/ids";
1717
import { foldAllBulk, unfoldAllBulk } from "@/core/codemirror/editing/commands";
1818
import { adaptiveLanguageConfiguration } from "@/core/codemirror/language/extension";
1919
import { OverridingHotkeyProvider } from "@/core/hotkeys/hotkeys";
@@ -24,7 +24,6 @@ import {
2424
exportedForTesting,
2525
flattenTopLevelNotebookCells,
2626
type NotebookState,
27-
SETUP_CELL_ID,
2827
} from "../cells";
2928
import {
3029
focusAndScrollCellIntoView,
@@ -2108,8 +2107,8 @@ describe("cell reducer", () => {
21082107
// Update the setup cell
21092108
actions.addSetupCellIfDoesntExist({ code: "# Updated setup code" });
21102109

2111-
// Check that the same setup cell was updated, not duplicated
2112-
expect(state.cellData[SETUP_CELL_ID].code).toBe("# Updated setup code");
2110+
// Check that the setup cell did not change, since it already exists
2111+
expect(state.cellData[SETUP_CELL_ID].code).toBe("# Setup code");
21132112
expect(state.cellData[SETUP_CELL_ID].edited).toBe(true);
21142113
expect(state.cellIds.inOrderIds).toContain(SETUP_CELL_ID);
21152114
});

frontend/src/core/cells/cells.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import type { CellConfig } from "../network/types";
2929
import { isRtcEnabled } from "../rtc/state";
3030
import { createDeepEqualAtom, store } from "../state/jotai";
3131
import { prepareCellForExecution, transitionCell } from "./cell";
32-
import { CellId } from "./ids";
32+
import { CellId, SCRATCH_CELL_ID, SETUP_CELL_ID } from "./ids";
3333
import { type CellLog, getCellLogsForMessage } from "./logs";
3434
import {
3535
focusAndScrollCellIntoView,
@@ -52,13 +52,6 @@ import {
5252
notebookQueueOrRunningCount,
5353
} from "./utils";
5454

55-
export const SCRATCH_CELL_ID = "__scratch__" as CellId;
56-
export const SETUP_CELL_ID = "setup" as CellId;
57-
58-
export function isSetupCell(cellId: CellId): boolean {
59-
return cellId === SETUP_CELL_ID;
60-
}
61-
6255
/**
6356
* The state of the notebook.
6457
*/
@@ -1335,8 +1328,7 @@ const {
13351328
code = "# Initialization code that runs before all other cells";
13361329
}
13371330

1338-
// First check if setup cell already exists
1339-
if (SETUP_CELL_ID in state.cellIds) {
1331+
if (state.cellIds.setupCellExists()) {
13401332
// Just focus on the existing setup cell
13411333
return {
13421334
...state,

frontend/src/core/cells/focus.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@ import type { EditorView } from "@codemirror/view";
44
import { atom, useAtomValue } from "jotai";
55
import { createReducerAndAtoms } from "@/utils/createReducer";
66
import type { CellConfig, RuntimeState } from "../network/types";
7-
import { type NotebookState, notebookAtom, SCRATCH_CELL_ID } from "./cells";
7+
import { type NotebookState, notebookAtom } from "./cells";
88
import type { CellId } from "./ids";
9+
import { SCRATCH_CELL_ID } from "./ids";
910
export interface CellFocusState {
1011
focusedCellId: CellId | null;
1112
lastFocusedCellId: CellId | null;

frontend/src/core/cells/ids.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ const uppercase = lowercase.toUpperCase();
99
const alphabet = lowercase + uppercase;
1010
const seen = new Set<CellId>();
1111

12+
export const SCRATCH_CELL_ID = "__scratch__" as CellId;
13+
export const SETUP_CELL_ID = "setup" as CellId;
14+
1215
/**
1316
* A typed CellId
1417
*/

frontend/src/core/codemirror/cells/extensions.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
import { closeCompletion, completionStatus } from "@codemirror/autocomplete";
44
import { type Extension, Prec } from "@codemirror/state";
55
import { EditorView, type KeyBinding, keymap } from "@codemirror/view";
6-
import { createTracebackInfoAtom, SCRATCH_CELL_ID } from "@/core/cells/cells";
7-
import { type CellId, HTMLCellId } from "@/core/cells/ids";
6+
import { createTracebackInfoAtom } from "@/core/cells/cells";
7+
import { type CellId, HTMLCellId, SCRATCH_CELL_ID } from "@/core/cells/ids";
88
import type { KeymapConfig } from "@/core/config/config-schema";
99
import type { HotkeyProvider } from "@/core/hotkeys/hotkeys";
1010
import { store } from "@/core/state/jotai";

0 commit comments

Comments
 (0)