PublicArchive/supabase
2
0
Fork 0
mirror of https://github.com/supabase/supabase.git synced 2026-06-30 12:29:05 -04:00
Code Issues Packages Projects Releases Wiki Activity
37,123 Commits 1,403 Branches 54 Tags
create-pull-request/patch
Commit Graph

6 Commits

Author SHA1 Message Date
Charis d153bab849 refactor(studio): extract SQL editor session store from god store (#47349) 
## What

PR 6 of the SQL editor state re-layering stack. Moves ephemeral,
never-persisted SQL editor state out of the snippet/folder "god store".

**Session store** — `state/sql-editor/sql-editor-session-state.ts` holds
per-snippet, read-by-many session state:
- query `results`
- `explainResults`
- the row `limit`

…with their mutators (`addResult`/`addResultError`/`resetResult`,
`addExplainResult`/`addExplainResultError`/`resetExplainResult`,
`resetResults`, `setLimit`). `removeSnippet` drops a snippet's session
entries via `clearForSnippet(id)`.

**Diff-request slice** — `state/sql-editor/sql-editor-diff-request.ts`.
The Assistant's "Insert code" / "Replace code" diff is *not* per-snippet
session state: it's a transient, fire-and-forget command produced
outside the editor (e.g. query blocks / assistant) and consumed exactly
once by whichever editor is active. It's modeled as a consume-once
request (`requestDiff` / `consumeDiffRequest`) rather than durable state
— the editor drains it on apply, so a stale diff can't leak into a later
editor or session. (Previously this was `diffContent` in the god store:
never cleared and triggered by object-reference identity.)

Consumers read session state from `useSqlEditorSessionSnapshot` and the
diff channel from `useSqlEditorDiffRequestSnapshot`, keeping
`useSqlEditorV2StateSnapshot` only for snippets/folders.

### Why not the TanStack Query cache for results/explain?

Editor execution is a **mutation**, not a keyed query — `mutation.data`
is per-hook-instance and not keyed by snippet id, and there's no caching
value to capture (re-running SQL must return *fresh* data, never a
cached result). `EXPLAIN ANALYZE` actually executes the statement, so a
declarative/auto-refetching `useQuery` is semantically wrong.
Results/explain are imperative mutation outputs, scoped to the session,
read by several decoupled consumers keyed by snippet id — exactly what a
small in-memory keyed store models honestly.

## Consumers migrated

- `SQLEditor.tsx` — results/explain/limit reads +
`addResult`/`addResultError`/`addExplainResult`/`addExplainResultError`/`setLimit`;
diff-apply effect now drains a consume-once request
- `UtilityPanel.tsx`, `UtilityTabResults.tsx`, `UtilityTabExplain.tsx`,
`UtilityActions.tsx`
- `QueryBlock/EditQueryButton.tsx` — produces via `requestDiff`

## Notes

- Result/explain types are kept verbatim from the god store
(pre-existing `any` row/error types come along unchanged; tightening
them is out of scope for this move).
- `ref()` on result rows is preserved to avoid Valtio proxying large row
sets.

## Tests

- `sql-editor-session-state.test.ts` — result/explain mutators,
`resetResults`, `clearForSnippet`, `limit`
- `sql-editor-diff-request.test.ts` — `requestDiff`,
`consumeDiffRequest` (drain + queue-of-one)

Validation:
- `pnpm --filter studio typecheck` ✅
- `pnpm exec vitest --run state/sql-editor/` ✅ (110 passed)
- lint ✅ (no new errors)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* SQL editor query results, EXPLAIN output, and the “Limit results to”
setting now persist more reliably across a session.
* AI-assisted SQL insert/replace actions now use a pending diff workflow
to apply updates more consistently.

* **Bug Fixes**
* Results/EXPLAIN rendering and downloads stay in sync with the latest
executed data.
* Switching databases/snippets now clears the correct temporary results.
* Diff application is more resilient when an editor is still loading,
including empty-vs-non-empty editor cases.

* **Tests**
  * Added coverage for the session and diff-request state logic.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Joshen Lim <joshenlimek@gmail.com>
 2026-06-30 11:24:48 -04:00
Charis 5cb81123ae refactor(studio): move SQL editor save trigger into a scheduler + provider (5/9) (#47316) 
## What

PR 5 of a stacked refactor. Moves *when to save* out of a module-load
`subscribe` and into an injectable **scheduler** armed by a headless
**provider**, splits the save queue, and adds an unsaved-close warning.

### Scheduler (`sql-editor-save-scheduler.ts`)
`createSaveScheduler({ state, saveMechanism, notify, getSaveMode })`
owns the save *policy*:
- **auto** mode drains the dirty snippet queue as edits land; **manual**
mode (the seam for a future opt-in; defaults to `auto`) leaves snippets
queued until `requestSave`. Folder saves always drain.
- `start()` returns an unsubscribe; `requestSave(id)` is the
explicit-save entry.

### Provider (`sql-editor-save-coordinator.tsx`)
Headless `SqlEditorSaveCoordinatorProvider` instantiates the mechanism
(invalidation via the **React Query client from context**, not the
global `getQueryClient`) + scheduler, `start()`s it in an effect
(start/stop with the provider), and exposes `requestSave` via
`useSqlEditorSaveCoordinator()`. Mounted in `ProjectContext` (under the
app's QueryClientProvider). Cmd+S and the SavingIndicator Retry now go
through `requestSave`.

### Queue split
`needsSaving` (snippets) and `pendingFolderSaves` (folders) are separate
queues, drained independently — the old snippet-vs-folder `if/else` is
gone.

### Unsaved-close warning
A `beforeunload` guard triggers the browser's native "Leave site?"
prompt while any snippet's `status !== 'saved'` (failed / in-flight /
never-saved).

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Improved SQL editor saving with a centralized save flow, including
automatic/manual save handling and immediate “Save Query” requests.
* Added unsaved-change detection so the app can warn before closing or
reloading when edits are still pending.

* **Bug Fixes**
* Retry actions now use the updated save flow for more reliable
re-saving.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-06-25 16:24:04 -04:00
Charis 16526bd6bf refactor(studio): extract SQL editor save mechanism + model folder lifecycle (4/9) (#47276) 
## What

PR 4 of a stacked refactor of the SQL editor snippet/folder state. It
pulls the persistence logic out of the store into an injectable
mechanism, and replaces the folder `'new-folder'` id sentinel with an
explicit lifecycle — plus a concurrency bug fix that surfaced along the
way.

### Save mechanism (`sql-editor-save.ts`)

`createSaveMechanism({ state, upsertContent, createSQLSnippetFolder,
updateSQLSnippetFolder, invalidate, notify, debounceMs })` → `{
saveSnippet, createFolder, updateFolder }`. The store's subscribe now
dispatches to it; *when* to save still lives in the subscribe (the
scheduler/provider move is PR 5). Per-id debounce cache lives in the
factory closure (no module-global leak).

- **`saveSnippet`** reads the live store snippet, guards
`isLoadedSnippet` so a content-less snippet can **never PUT an empty
body** (directly unit-tested), then builds the payload + drives status
transitions + gated invalidation.
- **`toast` is injected** as a `Notifier` (new generic DI contract in
`lib/notifier.ts`) — the mechanism no longer imports sonner.
- **create vs rename are two named-arg functions**, not an `isNew`
branch; rollback is deterministic per operation instead of matching on
`error.message` text.
- **caught errors are `unknown`**, narrowed via the existing
`getErrorMessage` util with a generic fallback — no `any`.

### Folder lifecycle (replaces the `NEW_FOLDER_ID` sentinel)

- **`FolderStatus`** enum (`new_editing | new_saving | editing | saving
| idle`) collapses the persistence and progress axes into one enum —
same pattern as `SnippetStatus` — with `isNewFolder` / `isFolderEditing`
/ `isFolderSaving` predicates. Tagging a folder as new/persisted is now
an explicit field, not an id convention.
- New placeholders get a **unique local id** (`crypto.randomUUID`);
`NEW_FOLDER_ID` is deleted, which also lifts the accidental
one-unsaved-folder-at-a-time limit.

### Bug fix: folder-rename rollback race

The shared `lastUpdatedFolderName` field let two in-flight renames
clobber each other's rollback target (and a shared `finally` could wipe
it). Replaced by a **per-folder `previousName`** on
`StateSnippetFolder`, so concurrent renames of different folders are
isolated. A new test runs two failing renames concurrently and asserts
each restores its own previous name.

## Tests

`sql-editor-save.test.ts` (mechanism — fakes + fake timers, incl.
content-less no-PUT and concurrent-rename isolation) and
folder-lifecycle predicate tests. `pnpm --filter studio typecheck`
clean; 82 state/sql-editor unit tests pass.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Improved SQL editor folder handling with clearer create, rename, and
save states.
* Added a more consistent notification flow for successful and failed
save actions.

* **Bug Fixes**
* Improved rollback handling when folder renames fail, helping restore
the previous name reliably.
* Updated save behavior to better protect against duplicate or
out-of-order updates.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-06-25 11:08:26 -04:00
Charis d5653f1f92 refactor(studio): unify snippet save + persistence into SnippetStatus (3/9) (#47251) 
## What

PR 3 of a stacked refactor of the SQL editor snippet state. Replaces the
two overlapping pieces of snippet lifecycle state — the `savingStates`
map (`IDLE|UPDATING|UPDATING_FAILED`) and the `isNotSavedInDatabaseYet`
boolean — with a single `SnippetStatus` enum.

## Status is attached at the data layer (never absent)

- `SnippetStatus` + `SnippetWithContent` now live in `data/content`. The
snippet queries attach `status: 'saved'` via a typed `withSavedStatus()`
helper, and `upsertContent` returns `SnippetWithContent` so move/rename
responses carry status too.
- A SQL-typed `getSqlSnippetById`/`useSqlSnippetByIdQuery` returns
`SnippetWithContent` (the generic `useContentIdQuery` stays for Reports,
which use it). `[id].tsx` loads content with **no casting**.
- `'new'` is attached on local creation (`createSqlSnippetSkeletonV2`).

## Behavior

Behavior-preserving for the existing auto-save flow (faithful mapping of
both old fields, including the replication-lag swallow). One incidental
fix: the read-only/saving indicator now also covers a brand-new
snippet's first save (previously only re-saves of persisted snippets had
distinct saving/failed states in some paths).

## Tests

New `sql-editor-lifecycle.test.ts` (29 tests) covering every predicate
and transition; existing rules tests updated. `pnpm --filter studio
typecheck` clean; 52 state/sql-editor unit tests pass.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

## Release Notes

* **Refactor**
* Restructured SQL snippet persistence tracking, replacing boolean flags
with a comprehensive status system for clearer visibility into save
progress.
* Enhanced saving indicator UI to reflect accurate snippet save states.

* **Tests**
* Added test coverage for snippet persistence state transitions and
lifecycle scenarios.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-06-24 08:56:39 -04:00
Charis e1e2498db0 refactor(studio): extract SQL editor domain rules into pure module (2/9) (#47204) 
## What

PR 2 of a stacked refactor of the SQL editor snippet state. **Stacked on
#47203 (PR 1)** — review/merge that first.

Extracts scattered business rules + the upsert-payload builder into a
new **pure** module `apps/studio/state/sql-editor/sql-editor-rules.ts`
(no Valtio, React, toast, or runtime data-layer imports):

- `canEditSnippet` — read-only rule (shared snippet you don't own), was
inline in `MonacoEditor` `disableEdit`
- `isSnippetOwner` — owner check, was inline in `ReadOnlyBadge` /
`SavingIndicator`
- `validateMoveToFolder` — 'shared snippet cannot be within a folder',
was a buried `toast.error`
- `buildUpsertPayload` — the PUT /content payload, was an inline object
literal (all `??` defaults preserved)
- `isLoadedSnippet` — type guard (see below)

## Bug fix: no more empty-content saves (and no non-null assertion)

The old payload builder used `{ ...content!, content_id: id }`. Tracing
that `!` upstream surfaced a real bug: **favoriting a snippet from the
sidebar that had never been opened** enqueued a save with no loaded
content, producing a PUT with an empty content body (rejected by API).

The requirement that a persisted snippet has loaded content is now
enforced **at the type level** rather than by a runtime assertion or
comment:
- `buildUpsertPayload` accepts only a `LoadedSnippet` (content
non-nullable) — the `!` is gone.
- the save subscriber crosses that boundary via the `isLoadedSnippet`
type guard.
- the sidebar favorite toggle loads content first (mirroring
`onSelectDuplicate` / the share modals), narrowing the fetched union
content to the SQL variant via its discriminant — **no type cast**.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved consistency in read-only behavior and ownership checks across
the SQL editor by centralizing permission logic.
* Fixed favorite toggle to ensure snippet content is fully loaded before
persisting changes.

* **Refactor**
* Centralized SQL snippet permission rules and validation logic into a
dedicated helper module.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-06-23 09:42:49 -04:00
Charis 2b24065c7b refactor(studio): relocate SQL editor store into state/sql-editor/ with facade (1/9) (#47203) 
## What

PR 1 of a stacked refactor that re-layers the SQL editor snippet state
(`apps/studio/state/sql-editor-v2.ts`). This first PR is a **pure
structural move with zero behavior change** — no consumer files are
touched.

- Relocates the Valtio store body into
`apps/studio/state/sql-editor/sql-editor-state.ts`
- Extracts the type declarations into
`apps/studio/state/sql-editor/types.ts`
- Adds `apps/studio/state/sql-editor/index.ts` as the public surface
- Keeps the old `apps/studio/state/sql-editor-v2.ts` path as a thin
re-export **facade**, so all existing importers keep working unchanged

## How to read the diff

`sql-editor/sql-editor-state.ts` (~507 lines) is the **verbatim
relocation** of the former `sql-editor-v2.ts` body — not new code. Git
does not show it as a rename because the old path is intentionally
retained as the facade. The only genuinely new lines are `types.ts`
(20), `index.ts` (8), and the facade itself (8).

## Why

The store has accreted four tangled responsibilities (snippet/folder
CRUD, query results, persistence, Assistant diff). The stack
incrementally splits these into pure rules, a persistent store, a
session store, and an injectable save mechanism whose trigger is a
swappable policy (setting up a future auto→manual save migration). Each
PR stays ≤300–400 non-test lines and behavior-preserving.

## Verification

- `pnpm --filter studio typecheck` passes (only pre-existing unrelated
module-resolution errors remain).
- Lint passes (no new errors).
- No consumer imports changed.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Refactor**
* Restructured SQL editor state management into a modular architecture
with improved separation of concerns and enhanced code organization.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-06-23 09:05:02 -04:00