fix(app): keep a closing tab's canvas off the next document (#958)

* fix(app): keep a closing tab's canvas off the next document

Tab components read the editor through useEditorStore, which resolves
to whichever tab is active at each call. When a tab closed, the next
tab became active before the closing canvas finished, so the canvas
registered its renderers with that document and its cleanup removed
them from the wrong one. Every closed document's renderers, WebGL
canvases, and detached editor DOM stayed in the remaining tab: about
1,200 DOM nodes, 320 listeners, and 2.5 MB of heap per open and close
in a production build.

EditorWorkspace now provides its tab's store to app components as it
already did for the SDK's useEditor, and useEditorStore returns it
inside the tab. App-level menus, shortcuts, and services outside any
tab keep following the active tab. Over eight open and close cycles,
DOM nodes, listeners, and the remaining tab's renderers stay flat.

* fix(app): bind the workspace itself to its tab's store

Vue resolves an injection from the parent, so EditorWorkspace could not
inject the store it provides and its own template kept following the
active tab, reading the next document while its tab closed. It now
takes its tab's store directly.

* refactor(app): pass the workspace its tab as a prop

EditorWorkspace read the active tab once during setup and relied on
WorkspaceView keying it by tab to keep that choice. WorkspaceView now
passes the tab, the workspace uses its store directly, and provides it
to the components below, so the binding is explicit in the template.

* test: keep the light-page setup from leaving documents unsaved

CanvasHelper switches a fresh document to Figma's light page so canvas
specs draw on it in the dark theme. Setting the page color counts as an
edit, so every spec's document started unsaved, and closing a tab asked
to save changes nobody made: both tab-close specs hit that dialog. The
store now exposes markDocumentSaved, which reload and save already use,
and the helper takes the light page as the saved state.

* test: wait for a closing menu before opening the next one

A menu stays in the page while it animates out, and a click on a menubar
trigger then is lost, so menu specs failed depending on which tests ran
before them. They now open menus through one helper that waits for the
previous menu to go and for the named menu to appear.

* test: configure storage through the current settings form

Storage settings moved the connection fields behind the provider row
and save them with one Save button, so the workspace spec timed out
looking for an Endpoint field that was not shown.

src/app/document/io/create.ts

@@ -93,6 +93,7 @@ export function createDocumentIOActions(
     openDOMFile,
     importDOMText,
     hasUnsavedChanges: sourceActions.hasUnsavedChanges,
+    markDocumentSaved: () => sourceActions.markDocumentSaved(),
     saveFigFile: sourceActions.saveFigFile,
     saveFigFileAs: sourceActions.saveFigFileAs
   }

src/app/editor/active-store/index.ts

@@ -1,4 +1,7 @@
-import { shallowRef, triggerRef, watch } from 'vue'
+import { hasInjectionContext, inject, provide, shallowRef, triggerRef, watch } from 'vue'
+import type { InjectionKey } from 'vue'
+
+import { provideEditor } from '@open-pencil/vue'
 
 import type { EditorStore } from '@/app/editor/session'
 
@@ -30,8 +33,24 @@ const storeProxy = new Proxy({} as EditorStore, {
   }
 })
 
+const TAB_STORE_KEY: InjectionKey<EditorStore> = Symbol('open-pencil-tab-store')
+
+/**
+ * Binds a tab's components to that tab's own store, for `useEditorStore` and the SDK's
+ * `useEditor`. Without it they would follow the active tab, so a closing tab's canvas would
+ * hand its renderers to the next document and never take them back.
+ */
+export function provideTabEditorStore(store: EditorStore) {
+  provide(TAB_STORE_KEY, store)
+  provideEditor(store)
+}
+
+/**
+ * The store of the tab this component belongs to, or the active tab's store outside any tab
+ * and outside component setup.
+ */
 export function useEditorStore(): EditorStore {
-  return storeProxy
+  return hasInjectionContext() ? inject(TAB_STORE_KEY, storeProxy) : storeProxy
 }
 
 /**

src/app/editor/session/modules.ts

@@ -78,6 +78,8 @@ export function createEditorStoreModules(
     setViewportSize: documentIO.setViewportSize,
     fitCurrentPageToViewport: documentIO.fitCurrentPageToViewport,
     hasUnsavedChanges: documentIO.hasUnsavedChanges,
+    /** Takes the document as it is now as its saved state, so closing it asks nothing. */
+    markDocumentSaved: documentIO.markDocumentSaved,
     saveFigFile: documentIO.saveFigFile,
     saveFigFileAs: documentIO.saveFigFileAs,
     getDocumentFilePath: documentIO.getDocumentFilePath,

src/components/editor/EditorWorkspace.vue

@@ -3,13 +3,13 @@ import { SplitterGroup, SplitterPanel, SplitterResizeHandle } from 'reka-ui'
 import { tv } from 'tailwind-variants'
 import { computed } from 'vue'
 
-import { formatShortcut, provideEditor, useI18n, useViewportKind } from '@open-pencil/vue'
+import { formatShortcut, useI18n, useViewportKind } from '@open-pencil/vue'
 
-import { useEditorStore } from '@/app/editor/active-store'
+import { provideTabEditorStore } from '@/app/editor/active-store'
 import { appRuntimeConfig } from '@/app/runtime/config'
 import { loadEditorLayout, saveEditorLayout } from '@/app/shell/layout-storage'
 import { appMenuShortcut } from '@/app/shell/menu/shortcut'
-import { activeTab } from '@/app/tabs'
+import type { Tab } from '@/app/tabs'
 import CanvasSplitRoot from '@/components/canvas/CanvasSplitRoot.vue'
 import CollabPanel from '@/components/collab-panel/CollabPanel.vue'
 import ActiveRoomOverlay from '@/components/collab-room/ActiveRoomOverlay.vue'
@@ -27,12 +27,14 @@ import splitterTheme from '@/theme/splitter'
 import WorkspacePill from './WorkspacePill.vue'
 
 const showChrome = appRuntimeConfig.showChrome
-const store = useEditorStore()
-// WorkspaceView keys this view by tab, so the tab's own store is fixed for its lifetime. Its
-// editor UI stays bound to that document rather than the app-level editor, which follows the
-// active tab and would move these subscriptions to the next document when this tab closes.
-const tab = activeTab.value
-if (tab) provideEditor(tab.store)
+/**
+ * The document tab this workspace edits. WorkspaceView keys the workspace by tab, so the tab and
+ * its store are fixed for its lifetime, and its editor UI stays bound to that document rather
+ * than following the active tab to the next document when this one closes.
+ */
+const { tab } = defineProps<{ tab: Tab }>()
+const store = tab.store
+provideTabEditorStore(store)
 const { isMobile } = useViewportKind()
 const initialEditorLayout = loadEditorLayout()
 const horizontalSplitterStyles = tv(splitterTheme)({ direction: 'horizontal' })
@@ -44,17 +46,13 @@ const { pending: roomPending } = useRoomActions()
 </script>
 
 <template>
-  <div
-    v-if="roomPending"
-    :key="'room-' + activeTab?.id"
-    class="relative flex flex-1 overflow-hidden"
-  >
+  <div v-if="roomPending" :key="'room-' + tab.id" class="relative flex flex-1 overflow-hidden">
     <ActiveRoomOverlay />
   </div>
 
   <SplitterGroup
     v-else-if="!isMobile && showChrome && store.state.showUI && !playingAlone"
-    :key="activeTab?.id"
+    :key="tab.id"
     direction="horizontal"
     class="flex-1 overflow-hidden"
     @layout="saveEditorLayout"
@@ -111,7 +109,7 @@ const { pending: roomPending } = useRoomActions()
 
   <div
     v-else-if="isMobile && showChrome && store.state.showUI"
-    :key="'mobile-' + activeTab?.id"
+    :key="'mobile-' + tab.id"
     class="flex flex-1 overflow-hidden"
   >
     <div class="relative flex min-w-0 flex-1">
@@ -123,11 +121,7 @@ const { pending: roomPending } = useRoomActions()
     <MobileDrawer />
   </div>
 
-  <div
-    v-else-if="showChrome"
-    :key="'collapsed-' + activeTab?.id"
-    class="flex flex-1 overflow-hidden"
-  >
+  <div v-else-if="showChrome" :key="'collapsed-' + tab.id" class="flex flex-1 overflow-hidden">
     <div class="relative flex min-w-0 flex-1">
       <EditorCanvas />
       <ActiveRoomOverlay />
@@ -149,7 +143,7 @@ const { pending: roomPending } = useRoomActions()
     </div>
   </div>
 
-  <div v-else :key="'bare-' + activeTab?.id" class="flex flex-1 overflow-hidden">
+  <div v-else :key="'bare-' + tab.id" class="flex flex-1 overflow-hidden">
     <div class="relative flex min-w-0 flex-1">
       <EditorCanvas />
       <ActiveRoomOverlay />

src/views/WorkspaceView.vue

@@ -231,6 +231,10 @@ onUnmounted(() => {
     <CommandPalette />
     <TabBar />
     <HomeWorkspace v-show="activeTab?.kind === 'home'" @new-document="createDocumentInCurrentTab" />
-    <EditorWorkspace v-if="activeTab && activeTab.kind !== 'home'" :key="activeTab.id" />
+    <EditorWorkspace
+      v-if="activeTab && activeTab.kind !== 'home'"
+      :key="activeTab.id"
+      :tab="activeTab"
+    />
   </div>
 </template>

tests/e2e/app/closed-documents.spec.ts

@@ -1,4 +1,4 @@
-import { expect, test } from '@playwright/test'
+import { expect, test, type Page } from '@playwright/test'
 
 import type * as AppTabs from '@/app/tabs'
 
@@ -7,9 +7,36 @@ import { testPath } from '#tests/helpers/paths'
 
 const FIXTURE = 'gold-preview.fig'
 
+async function openFixtureInNewTab(page: Page) {
+  await page.evaluate(async (fixture) => {
+    const tabsURL = '/src/app/tabs/index.ts'
+    const tabs: typeof AppTabs = await import(tabsURL)
+    const response = await fetch(`/__fixtures/${fixture}`)
+    await tabs.openFileInNewTab(new File([await response.arrayBuffer()], fixture))
+  }, FIXTURE)
+  await expect
+    .poll(() =>
+      page.evaluate(() => {
+        const store = window.openPencil?.getStore?.()
+        return Boolean(store && store.graph.nodes.size > 100 && !store.state.preparation)
+      })
+    )
+    .toBe(true)
+}
+
+function activeTabRenderers(page: Page) {
+  return page.evaluate(async () => {
+    const tabsURL = '/src/app/tabs/index.ts'
+    const tabs: typeof AppTabs = await import(tabsURL)
+    return tabs.getActiveStore().canvasRenderers.length
+  })
+}
+
 // A closed tab must release its document. CanvasKit's WebGL context table, an editor's global
 // text measurer, store effects created during a component's setup, and app-level event
-// subscriptions each kept every closed document alive: store, graph, canvas, and UI tree.
+// subscriptions each kept every closed document alive: store, graph, canvas, and UI tree. A
+// closing tab's canvas also registered its renderers with the tab that became active, which
+// kept every closed canvas and its whole UI tree alive in the remaining document.
 test('documents closed in tabs are released', async ({ page }) => {
   test.setTimeout(90_000)
   await page.route(`**/__fixtures/${FIXTURE}`, (route) =>
@@ -20,21 +47,13 @@ test('documents closed in tabs are released', async ({ page }) => {
   const cdp = await page.context().newCDPSession(page)
   await page.evaluate(() => Reflect.set(window, '__closedGraphs', []))
 
+  // The first document replaces the empty startup tab and stays open while others close.
+  await openFixtureInNewTab(page)
+  await expect.poll(() => activeTabRenderers(page)).toBeGreaterThan(0)
+  const remainingRenderers = await activeTabRenderers(page)
+
   for (let cycle = 0; cycle < 3; cycle++) {
-    await page.evaluate(async (fixture) => {
-      const tabsURL = '/src/app/tabs/index.ts'
-      const tabs: typeof AppTabs = await import(tabsURL)
-      const response = await fetch(`/__fixtures/${fixture}`)
-      await tabs.openFileInNewTab(new File([await response.arrayBuffer()], fixture))
-    }, FIXTURE)
-    await expect
-      .poll(() =>
-        page.evaluate(() => {
-          const store = window.openPencil?.getStore?.()
-          return Boolean(store && store.graph.nodes.size > 100 && !store.state.preparation)
-        })
-      )
-      .toBe(true)
+    await openFixtureInNewTab(page)
     await page.evaluate(async () => {
       const tabsURL = '/src/app/tabs/index.ts'
       const tabs: typeof AppTabs = await import(tabsURL)
@@ -59,4 +78,6 @@ test('documents closed in tabs are released', async ({ page }) => {
       { timeout: 15_000 }
     )
     .toBe(0)
+  // The remaining tab's canvas remounts when it becomes active again.
+  await expect.poll(() => activeTabRenderers(page)).toBe(remainingRenderers)
 })

tests/e2e/app/menu.spec.ts

@@ -2,6 +2,18 @@ import { expect, test, useEditorSetup } from '#tests/e2e/fixtures'
 
 const editor = useEditorSetup()
 
+/**
+ * Opens a top-level menu. A menu that just closed stays in the page while it animates out, and a
+ * click on a trigger then is lost, so this waits for it to go before clicking.
+ */
+async function openMenu(name: string) {
+  await expect(editor.page.getByRole('menu')).toHaveCount(0)
+  await editor.page.getByRole('menubar').getByRole('menuitem', { name, exact: true }).click()
+  const menu = editor.page.getByRole('menu', { name })
+  await expect(menu).toBeVisible()
+  return menu
+}
+
 test('menu bar is visible in browser mode', async () => {
   const menubar = editor.page.locator('[role="menubar"]')
   await expect(menubar).toBeVisible()
@@ -14,9 +26,7 @@ test('menu bar has all top-level menus', async () => {
 })
 
 test('File menu opens and shows items', async () => {
-  await editor.page.locator('[role="menubar"] [role="menuitem"]', { hasText: 'File' }).click()
-  const menu = editor.page.locator('[role="menu"]')
-  await expect(menu).toBeVisible()
+  const menu = await openMenu('File')
 
   const items = await menu.locator('[role="menuitem"]').allTextContents()
   expect(items.some((t) => t.includes('Open'))).toBe(true)
@@ -28,9 +38,7 @@ test('File menu opens and shows items', async () => {
 })
 
 test('Edit menu shows Undo/Redo/Delete', async () => {
-  await editor.page.locator('[role="menubar"] [role="menuitem"]', { hasText: 'Edit' }).click()
-  const menu = editor.page.locator('[role="menu"]')
-  await expect(menu).toBeVisible()
+  const menu = await openMenu('Edit')
 
   const items = await menu.locator('[role="menuitem"]').allTextContents()
   expect(items.some((t) => t.includes('Undo'))).toBe(true)
@@ -42,9 +50,7 @@ test('Edit menu shows Undo/Redo/Delete', async () => {
 })
 
 test('View menu shows zoom options', async () => {
-  await editor.page.locator('[role="menubar"] [role="menuitem"]', { hasText: 'View' }).click()
-  const menu = editor.page.locator('[role="menu"]')
-  await expect(menu).toBeVisible()
+  const menu = await openMenu('View')
 
   const items = await menu.locator('[role="menuitem"]').allTextContents()
   expect(items.some((t) => t.includes('Zoom to fit'))).toBe(true)
@@ -55,9 +61,7 @@ test('View menu shows zoom options', async () => {
 })
 
 test('Object menu shows Group/Ungroup/Component', async () => {
-  await editor.page.locator('[role="menubar"] [role="menuitem"]', { hasText: 'Object' }).click()
-  const menu = editor.page.locator('[role="menu"]')
-  await expect(menu).toBeVisible()
+  const menu = await openMenu('Object')
 
   const items = await menu.locator('[role="menuitem"]').allTextContents()
   expect(items.some((t) => t.includes('Group'))).toBe(true)
@@ -86,13 +90,13 @@ test('Move to page is disabled without a selection', async () => {
     store.switchPage(store.graph.getPages()[0].id)
   })
 
-  await editor.page.locator('[role="menubar"] [role="menuitem"]', { hasText: 'Object' }).click()
+  await openMenu('Object')
   const moveToPage = editor.page.getByRole('menuitem', { name: 'Move to page' })
   await expect(moveToPage).toHaveAttribute('data-disabled')
   await editor.page.keyboard.press('Escape')
 
   await editor.canvas.drawRect(200, 200, 100, 100)
-  await editor.page.locator('[role="menubar"] [role="menuitem"]', { hasText: 'Object' }).click()
+  await openMenu('Object')
   await expect(editor.page.getByRole('menuitem', { name: 'Move to page' })).not.toHaveAttribute(
     'data-disabled'
   )
@@ -111,7 +115,7 @@ test('Undo via Edit menu works', async () => {
   const beforeUndo = await getStoreStateNumber('selectedIds')
   expect(beforeUndo).toBe(1)
 
-  await editor.page.locator('[role="menubar"] [role="menuitem"]', { hasText: 'Edit' }).click()
+  await openMenu('Edit')
   const undoItem = editor.page.getByRole('menuitem', { name: /^Undo\b/ })
   await expect(undoItem).toBeEnabled()
   await undoItem.click()
@@ -120,7 +124,7 @@ test('Undo via Edit menu works', async () => {
   const afterUndo = await getStoreStateNumber('selectedIds')
   expect(afterUndo).toBe(0)
 
-  await editor.page.getByRole('menuitem', { name: 'Edit', exact: true }).click()
+  await openMenu('Edit')
   await expect(editor.page.getByRole('menuitem', { name: /^Redo\b/ })).toBeEnabled()
   await editor.page.keyboard.press('Escape')
 })
@@ -134,8 +138,8 @@ test('Duplicate via Edit menu works', async () => {
     return store.graph.getChildren(store.state.currentPageId).length
   })
 
-  await editor.page.locator('[role="menubar"] [role="menuitem"]', { hasText: 'Edit' }).click()
-  await editor.page.locator('[role="menu"] [role="menuitem"]', { hasText: 'Duplicate' }).click()
+  const editMenu = await openMenu('Edit')
+  await editMenu.locator('[role="menuitem"]', { hasText: 'Duplicate' }).click()
   await editor.canvas.waitForRender()
 
   const countAfter = await editor.page.evaluate(() => {
@@ -148,15 +152,15 @@ test('Duplicate via Edit menu works', async () => {
 })
 
 test('Zoom to fit via View menu works', async () => {
-  await editor.page.locator('[role="menubar"] [role="menuitem"]', { hasText: 'View' }).click()
-  await editor.page.locator('[role="menu"] [role="menuitem"]', { hasText: 'Zoom in' }).click()
+  const zoomInMenu = await openMenu('View')
+  await zoomInMenu.locator('[role="menuitem"]', { hasText: 'Zoom in' }).click()
   await editor.canvas.waitForRender()
 
   const zoomBefore = await getStoreStateNumber('zoom')
   expect(zoomBefore).toBeGreaterThan(1)
 
-  await editor.page.locator('[role="menubar"] [role="menuitem"]', { hasText: 'View' }).click()
-  await editor.page.locator('[role="menu"] [role="menuitem"]', { hasText: 'Zoom to fit' }).click()
+  const zoomToFitMenu = await openMenu('View')
+  await zoomToFitMenu.locator('[role="menuitem"]', { hasText: 'Zoom to fit' }).click()
   await editor.canvas.waitForRender()
 
   const zoomAfter = await getStoreStateNumber('zoom')

tests/e2e/storage/workspace.spec.ts

@@ -86,17 +86,13 @@ test('configured storage lists previews through ranges before opening the docume
   await page.keyboard.press('ControlOrMeta+KeyN')
   await page.getByRole('button', { name: 'Settings' }).last().click()
   await page.getByTestId('settings-section-storage').click()
-  await page.getByLabel('Endpoint').fill('https://s3.example.com')
-  await page.getByLabel('Bucket').fill('designs')
-
-  for (const [field, value] of [
-    ['access-key-id', 'access-key'],
-    ['secret-access-key', 'secret-key']
-  ] as const) {
-    const container = page.locator(`[data-credential="${field}"]`)
-    await container.locator('input').fill(value)
-    await container.getByRole('button', { name: 'Save' }).click()
-  }
+  const panel = page.getByTestId('settings-storage-panel')
+  await panel.getByRole('button', { name: /S3 storage/ }).click()
+  await panel.getByLabel('Endpoint').fill('https://s3.example.com')
+  await panel.getByLabel('Bucket').fill('designs')
+  await panel.getByLabel('Access key ID').fill('access-key')
+  await panel.getByLabel('Secret access key').fill('secret-key')
+  await page.getByRole('button', { name: 'Save', exact: true }).click()
 
   await page.getByTestId('settings-storage-open-workspace').click()
   await expect(page.getByTestId('recent-files-home')).toBeVisible()

tests/helpers/canvas.ts

@@ -39,7 +39,8 @@ export class CanvasHelper {
   /**
    * Canvas specs draw on Figma's light page. Playwright runs the app in its dark theme, where a new
    * document starts on Figma's dark page, so a fresh document (one empty page on that exact
-   * background) is switched back; any other document keeps its own.
+   * background) is switched back; any other document keeps its own. The switch is setup, not an
+   * edit, so the document stays saved and closing it asks nothing.
    * `tests/e2e/pages/theme-background.spec.ts` covers the theme.
    */
   async useLightPage() {
@@ -54,7 +55,9 @@ export class CanvasHelper {
         pages.length === 1 &&
         first.childIds.length === 0 &&
         [r, g, b].every((c) => c === darkDefault)
-      if (fresh) store.setPageColor({ r: 0.96, g: 0.96, b: 0.96, a: 1 })
+      if (!fresh) return
+      store.setPageColor({ r: 0.96, g: 0.96, b: 0.96, a: 1 })
+      store.markDocumentSaved()
     })
   }