refactor(sql-editor): finish EditorController/DiffController port (Step 2) (#48166)

## Summary
Step 2 of the SQL Editor testability plan.

`SQLEditorContext` already wrapped the Monaco refs and exposed a few
semantic imperative helpers (`getEditorSql`, `clearHighlights`,
`applyErrorHighlight`, `refocusEditor`, …). This finishes that
abstraction so no hook or controller touches
`editorRef.current`/`diffEditorRef.current` directly anymore — they only
call the port. The port is what will let Step 3's test harness inject a
real in-memory editor adapter instead of mocking Monaco; production
wires it to the real Monaco refs, unchanged.

- Extends the context value with two semantic controllers, backed by the
existing refs:
- `editor: EditorController` — `isReady`, `getValue`,
`getSelectionStartLine`, `getSql` (today's `getEditorSql`), `replaceAll`
(wraps the repeated `executeEdits(...)` pattern), `focus`,
`revealLineInCenter`, `highlightErrorLine` (today's
`applyErrorHighlight`), `clearHighlights`.
- `diff: DiffController` — `isMounted`, `getModifiedValue`, `setDiff`
(the diff-sync effect body), `attach` (today's `handleDiffEditorMount`).
- Migrates every touch point off raw refs onto the port:
`useSqlEditorExecution`, `usePrettifyQuery`, `useSqlEditorShortcuts`,
`SQLEditorControllers`' `readEditorSql`, and `useSqlEditorAi`'s
`acceptAiHandler`/`drainDiffRequest`/`handleDiffEditorMount`/diff-sync
effect.
- `SQLEditorEditorPanel.tsx` is intentionally left untouched — it wires
the raw refs into the real Monaco/DiffEditor React components for
rendering, which isn't decision logic to abstract.

Behavior-preserving.

## Test plan
- [x] `pnpm --filter studio typecheck`
- [x] `pnpm test:studio -- SQLEditor` (265 tests passing)
- [x] `pnpm --filter studio run lint:ratchet`
This commit is contained in:
Charis authored and GitHub committed 2026-07-21 16:22:47 -04:00
1 parent 1952abb6d1
commit 3d83e026f9
7 files changed
+175 -100

No files matched your search

@@ -1,4 +1,5 @@
import type { DiffOnMount, OnMount } from '@monaco-editor/react'
import type { UntrustedSqlFragment } from '@supabase/pg-meta'
import { Dispatch, SetStateAction } from 'react'
export interface SQLTemplate {
@@ -17,6 +18,34 @@ export type ContentDiff = {
modified: string
}
/**
* Semantic, Monaco-agnostic port onto the main editor. Hooks/controllers call
* this instead of touching `editorRef.current` directly, so the editor is
* swappable for a real in-memory adapter in tests without mocking.
*/
export type EditorController = {
isReady: () => boolean
getValue: () => string | undefined
getSelectionStartLine: () => number | undefined
getSql: (snippetContent?: UntrustedSqlFragment) => UntrustedSqlFragment | undefined
replaceAll: (text: string, source: string) => void
focus: () => void
revealLineInCenter: (line: number) => void
highlightErrorLine: (
error: { position?: unknown; formattedError?: string },
hasSelection: boolean
) => void
clearHighlights: () => void
}
/** Semantic, Monaco-agnostic port onto the diff editor. */
export type DiffController = {
isMounted: () => boolean
getModifiedValue: () => string | undefined
setDiff: (diff: ContentDiff, revealLine: number) => void
attach: (editor: IStandaloneDiffEditor) => void
}
export type SQLEditorContextValues = {
aiInput: string
setAiInput: Dispatch<SetStateAction<string>>
@@ -10,7 +10,13 @@ import {
type RefObject,
} from 'react'
import type { IStandaloneCodeEditor, IStandaloneDiffEditor } from './SQLEditor.types'
import type {
ContentDiff,
DiffController,
EditorController,
IStandaloneCodeEditor,
IStandaloneDiffEditor,
} from './SQLEditor.types'
import { computeErrorHighlightLine, getEditorSql } from './SQLEditor.utils'
type SQLEditorContextValue = {
@@ -26,14 +32,10 @@ type SQLEditorContextValue = {
markRefocusAfterRun: () => void
/** Refocus the editor iff a run-refocus was requested, then clear the flag. */
refocusEditorAfterRunIfNeeded: () => void
getEditorSql: (snippetContent?: UntrustedSqlFragment) => UntrustedSqlFragment | undefined
/** Clear any active error-highlight decorations. */
clearHighlights: () => void
/** Highlight and reveal the line referenced by an execute error's position. */
applyErrorHighlight: (
error: { position?: unknown; formattedError?: string },
hasSelection: boolean
) => void
/** Semantic port onto the main editor — no hook should touch `editorRef.current` directly. */
editor: EditorController
/** Semantic port onto the diff editor — no hook should touch `diffEditorRef.current` directly. */
diff: DiffController
}
const SQLEditorContext = createContext<SQLEditorContextValue | null>(null)
@@ -77,10 +79,34 @@ export const SQLEditorProvider = ({ children }: PropsWithChildren) => {
refocusEditor()
}, [refocusEditor])
const getEditorSqlFromEditor = useCallback((snippetContent?: UntrustedSqlFragment) => {
const editor = editorRef.current
if (!editor) return undefined
return getEditorSql(editor, snippetContent)
const isEditorReady = useCallback(() => editorRef.current !== null, [])
const getEditorValue = useCallback(() => editorRef.current?.getValue(), [])
const getSelectionStartLine = useCallback(
() => editorRef.current?.getSelection()?.startLineNumber,
[]
)
const getSqlFromEditor = useCallback((snippetContent?: UntrustedSqlFragment) => {
const editorInstance = editorRef.current
if (!editorInstance) return undefined
return getEditorSql(editorInstance, snippetContent)
}, [])
const replaceAll = useCallback((text: string, source: string) => {
const editorInstance = editorRef.current
const model = editorInstance?.getModel()
if (!editorInstance || !model) return
editorInstance.executeEdits(source, [{ text, range: model.getFullModelRange() }])
}, [])
const focusEditor = useCallback(() => {
editorRef.current?.focus()
}, [])
const revealLineInCenter = useCallback((line: number) => {
editorRef.current?.revealLineInCenter(line)
}, [])
const clearHighlights = useCallback(() => {
@@ -90,17 +116,19 @@ export const SQLEditorProvider = ({ children }: PropsWithChildren) => {
}
}, [])
const applyErrorHighlight = useCallback(
const highlightErrorLine = useCallback(
(error: { position?: unknown; formattedError?: string }, hasSelection: boolean) => {
if (!error.position || !monacoRef.current) return
const editor = editorRef.current
const editorInstance = editorRef.current
const monaco = monacoRef.current
const startLineNumber = hasSelection ? (editor?.getSelection()?.startLineNumber ?? 0) : 0
const startLineNumber = hasSelection
? (editorInstance?.getSelection()?.startLineNumber ?? 0)
: 0
const line = computeErrorHighlightLine(error, startLineNumber)
if (isNaN(line)) return
const decorations = editor?.deltaDecorations(
const decorations = editorInstance?.deltaDecorations(
[],
[
{
@@ -113,13 +141,69 @@ export const SQLEditorProvider = ({ children }: PropsWithChildren) => {
]
)
if (decorations) {
editor?.revealLineInCenter(line)
editorInstance?.revealLineInCenter(line)
lineHighlightsRef.current = decorations
}
},
[]
)
const editor = useMemo<EditorController>(
() => ({
isReady: isEditorReady,
getValue: getEditorValue,
getSelectionStartLine,
getSql: getSqlFromEditor,
replaceAll,
focus: focusEditor,
revealLineInCenter,
highlightErrorLine,
clearHighlights,
}),
[
isEditorReady,
getEditorValue,
getSelectionStartLine,
getSqlFromEditor,
replaceAll,
focusEditor,
revealLineInCenter,
highlightErrorLine,
clearHighlights,
]
)
const isDiffMounted = useCallback(() => diffEditorRef.current !== null, [])
const getModifiedValue = useCallback(
() => diffEditorRef.current?.getModel()?.modified.getValue(),
[]
)
const setDiff = useCallback((contentDiff: ContentDiff, revealLine: number) => {
const diffEditorInstance = diffEditorRef.current
const model = diffEditorInstance?.getModel()
if (!diffEditorInstance || !model || !model.original || !model.modified) return
model.original.setValue(contentDiff.original)
model.modified.setValue(contentDiff.modified)
diffEditorInstance.getModifiedEditor().revealLineInCenter(revealLine)
}, [])
const attachDiffEditor = useCallback((editorInstance: IStandaloneDiffEditor) => {
diffEditorRef.current = editorInstance
}, [])
const diff = useMemo<DiffController>(
() => ({
isMounted: isDiffMounted,
getModifiedValue,
setDiff,
attach: attachDiffEditor,
}),
[isDiffMounted, getModifiedValue, setDiff, attachDiffEditor]
)
const value = useMemo<SQLEditorContextValue>(
() => ({
editorRef,
@@ -130,18 +214,16 @@ export const SQLEditorProvider = ({ children }: PropsWithChildren) => {
clearPendingRunRefocus,
markRefocusAfterRun,
refocusEditorAfterRunIfNeeded,
getEditorSql: getEditorSqlFromEditor,
clearHighlights,
applyErrorHighlight,
editor,
diff,
}),
[
refocusEditor,
clearPendingRunRefocus,
markRefocusAfterRun,
refocusEditorAfterRunIfNeeded,
getEditorSqlFromEditor,
clearHighlights,
applyErrorHighlight,
editor,
diff,
]
)
@@ -111,7 +111,7 @@ export const useSqlEditorRun = () => useGuardedContext(RunContext, 'useSqlEditor
export const useSqlEditorUi = () => useGuardedContext(UiContext, 'useSqlEditorUi')
export const SQLEditorControllersProvider = ({ children }: PropsWithChildren) => {
const { monacoRef, scrollTopRef, getEditorSql: getEditorSqlFromEditor } = useSQLEditorContext()
const { monacoRef, scrollTopRef, editor } = useSQLEditorContext()
const { ref } = useParams()
const { data: project } = useSelectedProjectQuery()
@@ -148,8 +148,8 @@ export const SQLEditorControllersProvider = ({ children }: PropsWithChildren) =>
// to safety (acceptUntrustedSql) happens at each user-action site, never here.
const readEditorSql = useCallback((): UntrustedSqlFragment | undefined => {
const snippet = getSqlEditorV2StateSnapshot().snippets[id]
return getEditorSqlFromEditor(snippet?.snippet.content?.unchecked_sql)
}, [getEditorSqlFromEditor, id])
return editor.getSql(snippet?.snippet.content?.unchecked_sql)
}, [editor, id])
const { executeQuery, isExecuting, potentialIssues, resetPotentialIssues } =
useSqlEditorExecution({
@@ -1,6 +1,5 @@
import { useCallback } from 'react'
import { getEditorSql } from './SQLEditor.utils'
import { useSQLEditorContext } from './SQLEditorContext'
import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject'
import { formatSql } from '@/lib/formatSql'
@@ -14,7 +13,7 @@ import {
* the formatted SQL back to the snippet store. No-op while a diff is open.
*/
export function usePrettifyQuery({ id, isDiffOpen }: { id: string; isDiffOpen: boolean }) {
const { editorRef } = useSQLEditorContext()
const { editor } = useSQLEditorContext()
const { data: project } = useSelectedProjectQuery()
const snapV2 = useSqlEditorV2StateSnapshot()
@@ -25,21 +24,13 @@ export function usePrettifyQuery({ id, isDiffOpen }: { id: string; isDiffOpen: b
const state = getSqlEditorV2StateSnapshot()
const snippet = state.snippets[id]
if (editorRef.current && project) {
const editor = editorRef.current
const sql = getEditorSql(editor, snippet?.snippet.content?.unchecked_sql)
const formattedSql = formatSql(sql)
if (editor.isReady() && project) {
const sql = editor.getSql(snippet?.snippet.content?.unchecked_sql)
if (sql === undefined) return
const editorModel = editorRef?.current?.getModel()
if (editorRef.current && editorModel) {
editorRef.current.executeEdits('apply-prettify-edit', [
{
text: formattedSql,
range: editorModel.getFullModelRange(),
},
])
snapV2.setSql({ id, sql: formattedSql })
}
const formattedSql = formatSql(sql)
editor.replaceAll(formattedSql, 'apply-prettify-edit')
snapV2.setSql({ id, sql: formattedSql })
}
}, [editorRef, id, isDiffOpen, project, snapV2])
}, [editor, id, isDiffOpen, project, snapV2])
}
@@ -58,7 +58,7 @@ export function useSqlEditorAi({ id, editorMountCount, diff, prompt }: UseSqlEdi
} = diff
const { promptState, setPromptState, resetPrompt } = prompt
const { editorRef, diffEditorRef, refocusEditor } = useSQLEditorContext()
const { editor, diff: diffController, refocusEditor } = useSQLEditorContext()
const router = useRouter()
const { ref } = useParams()
@@ -131,25 +131,16 @@ export function useSqlEditorAi({ id, editorMountCount, diff, prompt }: UseSqlEdi
setIsAcceptDiffLoading(true)
// TODO: show error if undefined
if (!sourceSqlDiff || !editorRef.current || !diffEditorRef.current) return
if (!sourceSqlDiff || !editor.isReady() || !diffController.isMounted()) return
const editorModel = editorRef.current.getModel()
const diffModel = diffEditorRef.current.getModel()
if (!editorModel || !diffModel) return
const sql = diffModel.modified.getValue()
const sql = diffController.getModifiedValue()
if (sql === undefined) return
if (selectedDiffType === DiffType.NewSnippet) {
const { title } = await generateSqlTitle({ sql })
await handleNewQuery(sql, title)
} else {
editorRef.current.executeEdits('apply-ai-edit', [
{
text: sql,
range: editorModel.getFullModelRange(),
},
])
editor.replaceAll(sql, 'apply-ai-edit')
}
track('assistant_sql_diff_handler_evaluated', { handlerAccepted: true })
@@ -162,8 +153,8 @@ export function useSqlEditorAi({ id, editorMountCount, diff, prompt }: UseSqlEdi
setIsAcceptDiffLoading(false)
}
}, [
editorRef,
diffEditorRef,
editor,
diffController,
sourceSqlDiff,
selectedDiffType,
generateSqlTitle,
@@ -284,11 +275,11 @@ export function useSqlEditorAi({ id, editorMountCount, diff, prompt }: UseSqlEdi
)
const handleDiffEditorMount = useCallback(
(editor: IStandaloneDiffEditor) => {
diffEditorRef.current = editor
(mountedDiffEditor: IStandaloneDiffEditor) => {
diffController.attach(mountedDiffEditor)
setIsDiffEditorMounted(true)
},
[diffEditorRef]
[diffController]
)
const resetDiff = useEffectEvent(() => {
@@ -303,19 +294,14 @@ export function useSqlEditorAi({ id, editorMountCount, diff, prompt }: UseSqlEdi
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [id])
useEffect(() => {
const syncDiffEditor = useEffectEvent(() => {
if (isDiffOpen) {
const diffEditor = diffEditorRef.current
const model = diffEditor?.getModel()
if (model && model.original && model.modified) {
model.original.setValue(defaultSqlDiff.original)
model.modified.setValue(defaultSqlDiff.modified)
// scroll to the start line of the modification
const modifiedEditor = diffEditor!.getModifiedEditor()
const startLine = promptState.startLineNumber
modifiedEditor.revealLineInCenter(startLine)
}
diffController.setDiff(defaultSqlDiff, promptState.startLineNumber)
}
})
useEffect(() => {
syncDiffEditor()
// Temporary until we update eslint to ignore useEffectEvent
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [selectedDiffType, sourceSqlDiff])
@@ -323,21 +309,15 @@ export function useSqlEditorAi({ id, editorMountCount, diff, prompt }: UseSqlEdi
const request = diffRequest.pending
if (request === undefined) return
const editorModel = editorRef.current?.getModel()
// Editor isn't ready yet; leave the request pending. editorMountCount bumps
// on mount and re-runs this effect, so the request applies once mounted.
if (!editorModel) return
if (!editor.isReady()) return
const existingValue = editorRef.current?.getValue() ?? ''
const existingValue = editor.getValue() ?? ''
const plan = planDiffRequestApplication({ existingValue, request })
if (plan.kind === 'replace') {
// if the editor is empty, just copy over the code
editorRef.current?.executeEdits('apply-ai-message', [
{
text: plan.text,
range: editorModel.getFullModelRange(),
},
])
editor.replaceAll(plan.text, 'apply-ai-message')
} else {
setSourceSqlDiff(plan.diff)
setSelectedDiffType(plan.diffType)
@@ -358,11 +338,11 @@ export function useSqlEditorAi({ id, editorMountCount, diff, prompt }: UseSqlEdi
if (!isDiffOpen) {
setIsDiffEditorMounted(false)
setShowWidget(false)
} else if (diffEditorRef.current && isDiffEditorMounted) {
} else if (diffController.isMounted() && isDiffEditorMounted) {
setShowWidget(true)
return () => setShowWidget(false)
}
}, [diffEditorRef, isDiffOpen, isDiffEditorMounted])
}, [diffController, isDiffOpen, isDiffEditorMounted])
return useMemo(
() => ({
@@ -40,13 +40,7 @@ export function useSqlEditorExecution({
setAiTitle,
}: UseSqlEditorExecutionArgs) {
const { ref } = useParams()
const {
editorRef,
clearPendingRunRefocus,
refocusEditorAfterRunIfNeeded,
clearHighlights,
applyErrorHighlight,
} = useSQLEditorContext()
const { editor, clearPendingRunRefocus, refocusEditorAfterRunIfNeeded } = useSQLEditorContext()
const { data: project } = useSelectedProjectQuery()
const queryClient = useQueryClient()
@@ -80,7 +74,7 @@ export function useSqlEditorExecution({
},
onError(error: any, vars) {
if (id) {
applyErrorHighlight(error, hasSelection)
editor.highlightErrorLine(error, hasSelection)
sessionSnap.addResultError(id, error, vars.autoLimit)
}
@@ -95,7 +89,7 @@ export function useSqlEditorExecution({
return
}
if (editorRef.current === null || isExecuting || project === undefined) {
if (!editor.isReady() || isExecuting || project === undefined) {
clearPendingRunRefocus()
return
}
@@ -120,7 +114,7 @@ export function useSqlEditorExecution({
setAiTitle(id, sql)
}
clearHighlights()
editor.clearHighlights()
const impersonatedRoleState = getImpersonatedRoleState()
const connectionString = resolveConnectionString(
@@ -148,8 +142,7 @@ export function useSqlEditorExecution({
track('sql_editor_query_run_button_clicked')
},
[
editorRef,
clearHighlights,
editor,
clearPendingRunRefocus,
isDiffOpen,
id,
@@ -33,7 +33,7 @@ export function useSqlEditorShortcuts({
const os = detectOS()
const router = useRouter()
const { ref } = useParams()
const { editorRef, refocusEditor } = useSQLEditorContext()
const { editor, refocusEditor } = useSQLEditorContext()
const openNewSnippet = useCallback(() => {
if (!ref) return
@@ -67,7 +67,7 @@ export function useSqlEditorShortcuts({
case 'escape':
if (action.shouldDiscard) discardAiHandler()
resetPrompt()
editorRef.current?.focus()
editor.focus()
return
case 'none':
return
@@ -75,5 +75,5 @@ export function useSqlEditorShortcuts({
}
window.addEventListener('keydown', handler)
return () => window.removeEventListener('keydown', handler)
}, [editorRef, os, isDiffOpen, isPromptOpen, acceptAiHandler, discardAiHandler, resetPrompt])
}, [editor, os, isDiffOpen, isPromptOpen, acceptAiHandler, discardAiHandler, resetPrompt])
}