After a reload, the playground shows the default template's files under the selected saved instrument, and Save overwrites it #1737

Activity

1. joshunrau

   added

   BugType: existing behavior is wrongType: existing behavior is wrong

   Priority: HighSignificant or frequently hit user impact; target the next release or twoSignificant or frequently hit user impact; target the next release or two

   Difficulty: LowIsolated change, about 2 hours or lessIsolated change, about 2 hours or less

   Area: Playgroundapps/playground (in-browser instrument editor)apps/playground (in-browser instrument editor)

   on Oct 7, 2026
2. joshunrau commented on Oct 26, 2026

   

   joshunrau

   on Oct 26, 2026

   CollaboratorAuthor

   More actions

   Still reproduces on main (d86740e). Nothing has touched apps/playground/src/store since this was filed. No open PR touches store/index.ts; #1825 changes only the slices.

   The module-scope block also wipes IndexedDB

   store/index.ts:90-97 goes through persist's wrapped setState, which calls setItem. So before hydration resolves, it writes the pre-hydration state over app: the built-in instruments only, default settings and no _accessToken. Hydration still reads the old value, because idb-keyval creates the get transaction first. That is why the selector still shows Mine.

   Persist's own set after merge does not write. So app gets the saved instruments back only on the next persisting set: Monaco's mount (setEditorErrors, useEditorErrorSync.ts:33) or the first build. @/store is a static import of IndexPage.tsx, so all of this runs before its top-level await initialize(...) (IndexPage.tsx:14-17). The window covers the whole "Loading Editor and Toolchain" screen. Closing or reloading the tab there, or an esbuild-wasm init failure, loses every saved instrument and the login. This also came in with 7123b27. A node run of zustand 5.0.15's persist with an async in-memory storage shows app holding only the built-ins until the next set. The #1756 triage note flags the same write.

   The suggested fix needs two more pieces

   • Delete the block; do not keep it next to the new merge code. Give createEditorSlice an initial state built from defaultSelectedInstrument. A rejected hydration (IndexedDB unavailable) never calls merge, so without it the editor starts empty.
   • merge must not throw. The assignments setSelectedInstrument makes include resolveIndexFilename. It returns null only for an empty list, and throws when no file is index.* (packages/instrument-bundler/src/resolve.ts:16-18). Renaming index.ts to main.ts and clicking Save stores such a list. Persist's .catch swallows a throw in merge: there is no onRehydrateStorage, so nothing is logged and hasHydrated() stays false. The next set then writes the built-ins over app. Land the null-returning index lookup from the Renaming an open file in the playground leaves its tab on the old name and silently discards every later edit to it #1739 triage note first. Also add an onRehydrateStorage that logs the error, so a failed hydration is not silent.

   Plan

   Build on #1825. Generalise its loadDefaultInstrument(state) into an exported loadInstrumentIntoEditor(state, instrument) with setSelectedInstrument's null handling. Call it from setSelectedInstrument, removeInstrument, resetInstruments and merge; the #1767 triage note builds on the same helper. Then delete lines 90-97 and the resolveIndexFilename import. In "State and persistence" in apps/playground/AGENTS.md, say that merge loads the selected instrument into the editor, and that nothing may call setState at module scope, because every set writes to IndexedDB.

   Tests

   Add apps/playground/src/store/__tests__/index.test.ts. Mock idb-keyval with an async Map shared through vi.hoisted, and mock @/instruments as #1825's slice tests do. Seed the map, call vi.resetModules(), await import('@/store'), then await vi.waitFor(() => expect(useAppStore.persist.hasHydrated()).toBe(true)). Do not call persist.rehydrate(): on main the module-scope write has already replaced the seed, so it re-reads only the built-ins. An onFinishHydration listener added after the import never fires if hydration has already finished. Next to the issue's case:

   • it('should leave the stored saved instruments untouched until hydration finishes, so closing the tab while the toolchain loads loses nothing'), asserting on the map right after the import.
   • it('should hydrate a selected saved instrument with no index file, so one bad instrument cannot reset storage'): hasHydrated() is true, files is the instrument's and indexFilename is null.

   E2E in #1825's playground save button block of testing/src/specs/playground.spec.ts:

   1. Open a share link built from measureVisibilityInstrumentSource(title) and wait for preview.getByRole('heading', { name: title }).
   2. Call playground.goto() for the bare URL. Expect the same heading, and expect button:has(svg.lucide-save) (the locator fix: resolve 26 low-difficulty issues, one commit each #1825's Save test uses) to be disabled.

   Do not use page.reload(). The fragment would import the link again, and IndexPage.tsx:51-54 calls setSelectedInstrument on the existing copy, which hides the bug. The share link also avoids Create New Instrument, which has no accessible name until #1763.

   Order

   Land this before #1756, which changes the same persist options and plans a write gate on hasHydrated(). #1767 and #1745 build on it. Once merge loads files, the #1745 plan's hasHydrated() gate on the share import becomes required: a merge that lands after the import would replace the link's files too.

   _{Triage note written with Claude Code. Code references point at main @ d86740e.}