From 4fa106e53cbc6b25933571eafb9b5d7a5fed7988 Mon Sep 17 00:00:00 2001 From: Alaister Young Date: Mon, 29 Jun 2026 16:45:09 +0800 Subject: [PATCH] fix(studio): stop GraphiQL from corrupting other Monaco editors (#47363) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GraphiQL (`@graphiql/react`) runs a second Monaco instance that injects two global, page-wide styles which corrupt Studio's other editors once a GraphiQL chunk has loaded (it persists across client-side navigation, so a full reload hides it). After visiting GraphiQL and returning to e.g. the SQL editor, the editor collapses to a ~5px sliver and its syntax colors swap to GraphiQL's theme. **Changed:** - `monaco.css` — a higher-specificity counter-rule (`.monaco-editor.monaco-editor { position: relative !important }`) beats GraphiQL's runtime-injected `.monaco-editor { position: absolute !important }`, which otherwise pulls Studio's `@monaco-editor/react` wrapper out of flow and collapses it to ~5px. - GraphiQL now uses the primary `supabase` Monaco theme instead of a separate `supabase-graphql-*` theme, so the global `.mtk*` token palette stays identical and syntax colors no longer bleed into other editors. **Added:** - E2E test (`monaco-graphiql-coexistence.spec.ts`) reproducing both bugs via client-side SQL editor → GraphiQL → SQL editor navigation (a full reload unloads the chunk and hides the bug). - Component test (`CodeEditor.test.tsx`) guarding the height-class precedence regression from #47339/#47350 — a caller height (e.g. the email template editor's `h-96`) must win over the default `h-full`. Covered as a component test since the email source editor isn't reachable on self-hosted. ## To test - Open the SQL editor → **Integrations → GraphiQL** → back to the SQL editor (in-app navigation, not a reload). It should stay full height and keep its own syntax colors. - Confirm autocomplete still works in the SQL editor. - `pnpm --prefix e2e/studio run e2e -- features/monaco-graphiql-coexistence.spec.ts` - `pnpm --prefix apps/studio test -- CodeEditor.test` ## Summary by CodeRabbit * **Bug Fixes** * Improved Monaco editor styling so GraphiQL no longer affects the SQL editor’s theme or layout when navigating between them. * Fixed editor sizing so a custom height now takes precedence over the default full-height setting. * Polished GraphiQL panel styling for more consistent spacing and appearance across themes. * **New Features** * GraphiQL now uses the shared editor theme for better visual consistency with Studio. --------- Co-authored-by: Alaister Young <10985857+alaister@users.noreply.github.com> Co-authored-by: Joshen Lim --- .../Integrations/GraphQL/GraphiQLTab.tsx | 94 +++++--------- .../Integrations/GraphQL/graphiql-styles.css | 19 +++ .../Integrations/GraphQL/graphiql.module.css | 25 ++-- .../ui/CodeEditor/CodeEditor.test.tsx | 38 ++++++ .../components/ui/CodeEditor/CodeEditor.tsx | 10 +- .../ui/CodeEditor/CodeEditor.utils.ts | 24 +++- .../monaco-graphiql-coexistence.spec.ts | 120 ++++++++++++++++++ 7 files changed, 240 insertions(+), 90 deletions(-) create mode 100644 apps/studio/components/interfaces/Integrations/GraphQL/graphiql-styles.css create mode 100644 apps/studio/components/ui/CodeEditor/CodeEditor.test.tsx create mode 100644 e2e/studio/features/monaco-graphiql-coexistence.spec.ts diff --git a/apps/studio/components/interfaces/Integrations/GraphQL/GraphiQLTab.tsx b/apps/studio/components/interfaces/Integrations/GraphQL/GraphiQLTab.tsx index 6c761321c57..b5a29d8f9e9 100644 --- a/apps/studio/components/interfaces/Integrations/GraphQL/GraphiQLTab.tsx +++ b/apps/studio/components/interfaces/Integrations/GraphQL/GraphiQLTab.tsx @@ -1,5 +1,5 @@ -import 'graphiql/style.css' import 'graphiql/setup-workers/webpack' +import './graphiql-styles.css' import { useMonaco, type GraphiQLPlugin } from '@graphiql/react' import { createGraphiQLFetcher, Fetcher } from '@graphiql/toolkit' @@ -8,7 +8,7 @@ import { useParams } from 'common' import { GraphiQL, HISTORY_PLUGIN } from 'graphiql' import { User as IconUser } from 'lucide-react' import { useTheme } from 'next-themes' -import { useCallback, useEffect, useMemo, useRef, useState, type RefObject } from 'react' +import { useCallback, useEffect, useMemo, useState } from 'react' import { toast } from 'sonner' import { LogoLoader } from 'ui' @@ -19,6 +19,7 @@ import { IntrospectionEnabledNotice } from './IntrospectionEnabledNotice' import { usePgGraphqlIntrospectionStatus } from './usePgGraphqlIntrospectionStatus' import { getTheme } from '@/components/interfaces/App/MonacoThemeProvider' import { RoleImpersonationSelector } from '@/components/interfaces/RoleImpersonationSelector' +import { BASE_MONACO_EDITOR_OPTIONS } from '@/components/ui/CodeEditor/CodeEditor.utils' import { useSessionAccessTokenQuery } from '@/data/auth/session-access-token-query' import { useProjectPostgrestConfigQuery } from '@/data/config/project-postgrest-config-query' import { useAsyncCheckPermissions } from '@/hooks/misc/useCheckPermissions' @@ -33,80 +34,46 @@ const ROLE_IMPERSONATION_PLUGIN: GraphiQLPlugin = { content: () => , } -const MONACO_THEME = { dark: 'supabase-graphql-dark', light: 'supabase-graphql-light' } - -const GraphiQLMonacoTheme = ({ resolvedTheme }: { resolvedTheme: 'dark' | 'light' }) => { - const { monaco } = useMonaco() - - useEffect(() => { - if (!monaco) return - const dark = getTheme('dark') - const light = getTheme('light') - monaco.editor.defineTheme(MONACO_THEME.dark, { - ...dark, - rules: [...dark.rules, { token: 'argument.identifier.gql', foreground: '908aff' }], - }) - monaco.editor.defineTheme(MONACO_THEME.light, { - ...light, - rules: [...light.rules, { token: 'argument.identifier.gql', foreground: '6c69ce' }], - // Match the dashboard's bg-default in light mode so the editor doesn't read - // as a darker square against the surrounding UI. - colors: { ...light.colors, 'editor.background': '#fcfcfc' }, - }) - monaco.editor.setTheme(MONACO_THEME[resolvedTheme]) - }, [monaco, resolvedTheme]) - - return null -} - /** - * Inset the editor content from the container edges without floating the scroll shadow. - * - * Monaco anchors its `.scroll-decoration` (the shadow shown when scrolled) to the editor's - * top edge and absolutely positions the line numbers at the gutter's left edge, so container - * padding can't move either — it would just push the whole editor (shadow included) inward. - * Instead use Monaco's own options: - * - `padding` insets the content top/bottom while leaving the shadow pinned to the top, so - * `.graphiql-query-editor` can stay full-bleed (`p-0`) for a flush shadow. - * - `glyphMargin` reserves an empty column to the left of the line numbers (same gutter - * background), giving them left breathing room. - * - * GraphiQL shares the global Monaco instance with the rest of Studio (the SQL editor etc.), - * so we must scope `updateOptions` to editors that live inside the GraphiQL container — - * `monaco.editor.getEditors()`/`onDidCreateEditor()` see every editor in the app, and - * applying these options globally would shift the SQL editor's layout too. Filtering by - * container also means there's nothing to revert on unmount: we never touch other editors. + * GraphiQL (@graphiql/react) bundles its own Monaco instance, separate from the AMD-loaded one + * the rest of Studio uses — they only share the global `.monaco-*` CSS class names, not JS state. + * So theme/options applied here via @graphiql/react's `useMonaco` touch GraphiQL's editors only + * and never leak onto Studio's editors */ -const GraphiQLEditorOptions = ({ - containerRef, -}: { - containerRef: RefObject -}) => { +const GraphiQLEditorSettings = ({ theme }: { theme: 'dark' | 'light' }) => { const { monaco } = useMonaco() useEffect(() => { if (!monaco) return - const options = { padding: { top: 16, bottom: 16 }, glyphMargin: true } - const applyToGraphiQLEditor = (editor: ReturnType[number]) => { - if (containerRef.current?.contains(editor.getContainerDomNode())) { - editor.updateOptions(options) - } - } + monaco.editor.defineTheme('supabase', getTheme(theme)) + monaco.editor.setTheme('supabase') + }, [monaco, theme]) - monaco.editor.getEditors().forEach(applyToGraphiQLEditor) - const disposable = monaco.editor.onDidCreateEditor(applyToGraphiQLEditor) + useEffect(() => { + if (!monaco) return + const options = { + ...BASE_MONACO_EDITOR_OPTIONS, + padding: { top: 16, bottom: 16 }, + glyphMargin: true, + } + const applyToEditor = (editor: ReturnType[number]) => + editor.updateOptions(options) + + monaco.editor.getEditors().forEach(applyToEditor) + const disposable = monaco.editor.onDidCreateEditor(applyToEditor) return () => disposable.dispose() - }, [monaco, containerRef]) + }, [monaco]) return null } export const GraphiQLTab = () => { - const editorContainerRef = useRef(null) - const { resolvedTheme } = useTheme() const { ref: projectRef } = useParams() + + const { resolvedTheme } = useTheme() const currentTheme = resolvedTheme?.includes('dark') ? 'dark' : 'light' + const { data: accessToken } = useSessionAccessTokenQuery({ enabled: IS_PLATFORM }) const { data: project } = useSelectedProjectQuery() @@ -188,8 +155,7 @@ export const GraphiQLTab = () => { return (
- - + {notice === 'opt-in' && ( { onDisabled={handleIntrospectionChanged} /> )} -
+
diff --git a/apps/studio/components/interfaces/Integrations/GraphQL/graphiql-styles.css b/apps/studio/components/interfaces/Integrations/GraphQL/graphiql-styles.css new file mode 100644 index 00000000000..7d157c90be9 --- /dev/null +++ b/apps/studio/components/interfaces/Integrations/GraphQL/graphiql-styles.css @@ -0,0 +1,19 @@ +/* + * GraphiQL v5 bundles a full copy of Monaco's CSS into its stylesheet. Studio's + * SQL editor (and every other Monaco instance) shares the same global Monaco + * instance, and those `.monaco-*` rules are unscoped, so importing GraphiQL's + * stylesheet directly leaks its Monaco theme onto the SQL editor — whichever + * stylesheet happens to load last wins the cascade. + * + * Importing it into a low-priority cascade layer fixes this without touching the + * vendored bundle: Studio's own Monaco styles (the runtime-injected monaco-editor + * CSS and styles/monaco.css) are unlayered, and unlayered rules always beat any + * layered rule. So Studio's Monaco styling wins everywhere — including inside + * GraphiQL's editor, which is the consistent behavior we want. + * + * GraphiQL's own UI (the `.graphiql-container` selectors) still applies: the + * `graphiql` layer is registered after Tailwind's layers (this file loads after + * globals.css), so it outranks Tailwind's base/component resets just as the + * unlayered import did before. + */ +@import 'graphiql/style.css' layer(graphiql); diff --git a/apps/studio/components/interfaces/Integrations/GraphQL/graphiql.module.css b/apps/studio/components/interfaces/Integrations/GraphQL/graphiql.module.css index 359b954498a..46c71de786d 100644 --- a/apps/studio/components/interfaces/Integrations/GraphQL/graphiql.module.css +++ b/apps/studio/components/interfaces/Integrations/GraphQL/graphiql.module.css @@ -24,6 +24,7 @@ .root :global(.graphiql-horizontal-drag-bar) { @apply border-l border-default; + background-color: var(--graphiql-response-bg); } .root :global(.graphiql-horizontal-drag-bar:hover::after) { @@ -63,17 +64,10 @@ .root :global(.graphiql-response .monaco-editor), .root :global(.graphiql-response .monaco-editor .margin), .root :global(.graphiql-response .monaco-editor .monaco-editor-background) { + @apply pt-0; background-color: var(--graphiql-response-bg) !important; } -/* Editor-to-response drag bar: blend it into the response side so it doesn't read as - * a third color sandwiched between the two panes. The plugin-to-main drag bar is left - * alone — it sits next to the editor surface and our default border there is fine. */ -.root :global(.graphiql-editors + .graphiql-horizontal-drag-bar) { - background-color: var(--graphiql-response-bg); - border-left: 0; -} - .root :global(.graphiql-button), .root :global(button.graphiql-button), .root :global(.graphiql-un-styled), @@ -105,21 +99,20 @@ --border-radius-8: 0; --border-radius-12: 0; --popover-box-shadow: none; + /* Theme-independent: the Supabase green, and a response surface whose own token + * (--background-surface-300) already resolves per theme — so only --color-base below + * needs to branch on light/dark. */ + --color-primary: 153, 50%, 50%; + --graphiql-response-bg: hsl(var(--background-surface-300)); } :global(body.graphiql-dark) .root { /* HSL form of #1f1f1f, matches the editor.background returned by getTheme('dark') */ --color-base: 0, 0%, 12%; - --color-primary: 153, 50%, 50%; - --graphiql-response-bg: hsl(var(--background-surface-300)); } :global(body.graphiql-light) .root { - /* HSL form of #fcfcfc, matches the dashboard bg-default and the editor.background - * we override to in GraphiQLTab.tsx so the editor reads as part of the surrounding UI. */ + /* HSL form of #fcfcfc, matches the dashboard bg-default so the editor reads as part + * of the surrounding UI. */ --color-base: 0, 0%, 98.8%; - --color-primary: 153, 50%, 50%; - /* Drop the response below the editor in light mode (inverse of dark) so it reads as - * the lower / "data" surface rather than the lifted one. */ - --graphiql-response-bg: hsl(var(--background-surface-300)); } diff --git a/apps/studio/components/ui/CodeEditor/CodeEditor.test.tsx b/apps/studio/components/ui/CodeEditor/CodeEditor.test.tsx new file mode 100644 index 00000000000..d4f6d10fea7 --- /dev/null +++ b/apps/studio/components/ui/CodeEditor/CodeEditor.test.tsx @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest' + +import { CodeEditor } from './CodeEditor' +import { render } from '@/tests/helpers' + +/** + * CodeEditor applies a default `h-full` so editors with no explicit height fill their container + * (GraphiQL, etc.). Callers that DO set a height (e.g. the email template editor's `h-96`) must + * win — otherwise the editor collapses to a single line. + * + * This regressed once already: #47339 appended the default as `cn(className, 'monaco-editor', + * 'h-full')`, and tailwind-merge keeps the *last* conflicting height utility, so the trailing + * `h-full` clobbered caller heights. #47350 fixed it by passing `className` last. These tests + * lock that ordering in. + * + * The `
` that receives the class is rendered by @monaco-editor/react before + * Monaco loads, so it's present in jsdom without a working editor. + */ +describe('CodeEditor height class precedence', () => { + const getEditorEl = (container: HTMLElement) => container.querySelector('.monaco-editor') + + it('lets a caller-supplied height win over the default h-full (regression #47350)', () => { + const { container } = render() + + const editor = getEditorEl(container) + expect(editor, 'editor wrapper should render').toBeTruthy() + expect(editor).toHaveClass('h-96') + expect(editor, 'default h-full must not override the caller height').not.toHaveClass('h-full') + }) + + it('falls back to the default h-full when the caller sets no height', () => { + const { container } = render() + + const editor = getEditorEl(container) + expect(editor, 'editor wrapper should render').toBeTruthy() + expect(editor).toHaveClass('h-full') + }) +}) diff --git a/apps/studio/components/ui/CodeEditor/CodeEditor.tsx b/apps/studio/components/ui/CodeEditor/CodeEditor.tsx index 96e263d54ae..f912bdf6e49 100644 --- a/apps/studio/components/ui/CodeEditor/CodeEditor.tsx +++ b/apps/studio/components/ui/CodeEditor/CodeEditor.tsx @@ -6,7 +6,7 @@ import { RefObject, useEffect, useRef, useState } from 'react' import { cn } from 'ui' import { useSetCommandMenuOpen } from 'ui-patterns/CommandMenu' -import { alignEditor } from './CodeEditor.utils' +import { alignEditor, BASE_MONACO_EDITOR_OPTIONS } from './CodeEditor.utils' import { Markdown } from '@/components/interfaces/Markdown' import { useLatest } from '@/hooks/misc/useLatest' import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject' @@ -109,19 +109,13 @@ export const CodeEditor = ({ const optionsMerged = merge( { - tabSize: 2, - fontSize: 13, + ...BASE_MONACO_EDITOR_OPTIONS, domReadOnly: isReadOnly, readOnly: isReadOnly, - minimap: { enabled: false }, - wordWrap: 'on', - fixedOverflowWidgets: true, - contextmenu: true, lineNumbers: hideLineNumbers ? 'off' : undefined, glyphMargin: hideLineNumbers ? false : undefined, lineNumbersMinChars: hideLineNumbers ? 0 : 4, folding: hideLineNumbers ? false : undefined, - scrollBeyondLastLine: false, }, options ) diff --git a/apps/studio/components/ui/CodeEditor/CodeEditor.utils.ts b/apps/studio/components/ui/CodeEditor/CodeEditor.utils.ts index 5540d9d1456..a33b18bf56e 100644 --- a/apps/studio/components/ui/CodeEditor/CodeEditor.utils.ts +++ b/apps/studio/components/ui/CodeEditor/CodeEditor.utils.ts @@ -1,6 +1,26 @@ -export const alignEditor = (editor: any) => { +import type { editor } from 'monaco-editor' + +/** + * Base Monaco editor options shared across Studio's editors so font size, indentation and + * chrome stay consistent everywhere a Monaco editor is rendered. CodeEditor layers per-instance + * options (read-only, line numbers) on top; GraphiQL (which runs its own Monaco instance) layers + * its own padding/glyphMargin on top via `editor.updateOptions`. + * + * Keep this as the single source of truth — don't redeclare these values at call sites. + */ +export const BASE_MONACO_EDITOR_OPTIONS: editor.IStandaloneEditorConstructionOptions = { + tabSize: 2, + fontSize: 13, + minimap: { enabled: false }, + wordWrap: 'on', + fixedOverflowWidgets: true, + contextmenu: true, + scrollBeyondLastLine: false, +} + +export const alignEditor = (editor: editor.IStandaloneCodeEditor) => { // Add margin above first line - editor.changeViewZones((accessor: any) => { + editor.changeViewZones((accessor) => { accessor.addZone({ afterLineNumber: 0, heightInPx: 4, diff --git a/e2e/studio/features/monaco-graphiql-coexistence.spec.ts b/e2e/studio/features/monaco-graphiql-coexistence.spec.ts new file mode 100644 index 00000000000..a03a15cd253 --- /dev/null +++ b/e2e/studio/features/monaco-graphiql-coexistence.spec.ts @@ -0,0 +1,120 @@ +import { expect, type Page } from '@playwright/test' + +import { env } from '../env.config.js' +import { test } from '../utils/test.js' +import { toUrl } from '../utils/to-url.js' + +/** + * Studio's editors (@monaco-editor/react) and GraphiQL (@graphiql/react) historically ran as + * two separate Monaco instances on one page. Each injects Monaco's global CSS (.monaco-editor + * layout rules + .mtk* theme classes), and they collide: after visiting GraphiQL the SQL + * editor's Monaco wrapper got reflowed to a ~5px slit (covered by the background) and its + * syntax colors swapped to GraphiQL's theme. + * + * IMPORTANT: the bug only reproduces with *client-side* (SPA) navigation. A full `page.goto` + * reload unloads GraphiQL's CSS chunk, which removes the offending injected rule — so these + * tests navigate via the Next router to keep the chunk loaded, exactly like a real user. + */ +test.describe('Monaco / GraphiQL coexistence', () => { + test.skip(env.IS_PLATFORM, 'Self-hosted mode only — GraphiQL + SQL editor on one local project') + + // Client-side navigation via the Next router (keeps already-loaded chunks/CSS in place). + const routerPush = async (page: Page, path: string) => { + await page.evaluate((p) => { + const router = (window as unknown as { next?: { router?: { push: (p: string) => void } } }) + .next?.router + if (!router) throw new Error('Next router not available for client-side navigation') + router.push(p) + }, path) + } + + const firstEditorHeight = async (page: Page) => { + const box = await page.locator('.monaco-editor').first().boundingBox() + return box?.height ?? 0 + } + + const openGraphiQL = async (page: Page, ref: string) => { + await routerPush(page, `/project/${ref}/integrations/graphiql/graphiql`) + await expect(page.locator('.graphiql-container'), 'GraphiQL container should load').toBeVisible( + { timeout: 30000 } + ) + // GraphiQL mounting its Monaco editor is what injects the global rule that breaks others. + await expect( + page.locator('.graphiql-query-editor .monaco-editor').first(), + 'GraphiQL query editor (Monaco) should mount' + ).toBeVisible({ timeout: 30000 }) + } + + test('SQL editor stays full height after visiting GraphiQL', async ({ page, ref }) => { + // Baseline: a freshly-loaded SQL editor fills its pane. + await page.goto(toUrl(`/project/${ref}/sql/new?skip=true`)) + await expect( + page.locator('.monaco-editor').first(), + 'SQL editor Monaco should render' + ).toBeVisible({ timeout: 30000 }) + await expect.poll(() => firstEditorHeight(page), { timeout: 15000 }).toBeGreaterThan(100) + + // Client-side: visit GraphiQL, then come back to the SQL editor. + await openGraphiQL(page, ref) + await routerPush(page, `/project/${ref}/sql/new?skip=true`) + await expect(page, 'should navigate back to SQL editor').toHaveURL(/\/sql\//, { + timeout: 30000, + }) + await page.locator('.monaco-editor').first().waitFor({ state: 'attached', timeout: 30000 }) + + // With the bug the wrapper collapses to ~5px. It must stay full height. + await expect + .poll(() => firstEditorHeight(page), { + timeout: 15000, + message: 'SQL editor must not collapse to a sliver after visiting GraphiQL', + }) + .toBeGreaterThan(100) + }) + + test('SQL editor keeps its own theme after visiting GraphiQL', async ({ page, ref }) => { + await page.goto(toUrl(`/project/${ref}/sql/new?skip=true`)) + await expect( + page.locator('.monaco-editor').first(), + 'SQL editor Monaco should render' + ).toBeVisible({ timeout: 30000 }) + + await openGraphiQL(page, ref) + await routerPush(page, `/project/${ref}/sql/new?skip=true`) + await expect(page, 'should navigate back to SQL editor').toHaveURL(/\/sql\//, { + timeout: 30000, + }) + + // Monaco's token colors are global `.mtk*` classes. GraphiQL's gql-argument color (#6c69ce) + // must never enter that palette, or every other editor gets rethemed. + await expect + .poll( + () => + page.evaluate(() => { + const gqlColor = 'rgb(108, 105, 206)' // #6c69ce — GraphiQL's argument.identifier.gql + for (const sheet of Array.from(document.styleSheets)) { + let rules: CSSRuleList + try { + rules = sheet.cssRules + } catch { + continue + } + for (const rule of Array.from(rules)) { + const styleRule = rule as CSSStyleRule + if ( + /^\.mtk\d+$/.test(styleRule.selectorText || '') && + styleRule.style?.color === gqlColor + ) { + return true + } + } + } + return false + }), + { + timeout: 15000, + message: "GraphiQL's theme must not bleed into the SQL editor's token colors", + } + ) + .toBe(false) + }) +})