From 2ae37e552a614ef0b2f62dee34d2b83499092d6f Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Wed, 1 Jul 2026 15:53:48 +0000 Subject: [PATCH] fix: app.doc is dead; app.document is never assigned (#106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createApp built the app object with a `doc` field, but every other module (explain-graph.js, results.js, schema-detail.js, file-menu.js, shortcuts.js, app.js itself) read app.document instead — never assigned, so `app.document || document` silently always fell through to the global document. Rename app.doc -> app.document and drop the fallbacks that were provably unreachable; leave the ones intentionally tolerant of a null/minimal app (detached-view.js, explain-graph.js, schema-detail.js, and shortcuts.js — which has its own delete-app.document test) untouched. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01784dKCpk5W7rdpUAwcQnix --- CHANGELOG.md | 11 +++++++++++ src/ui/app.js | 8 ++++---- src/ui/file-menu.js | 4 ++-- src/ui/results.js | 6 +++--- tests/unit/app.test.js | 6 ++++++ 5 files changed, 26 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e08f6a6..8b15c14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -213,6 +213,17 @@ auto-generated per-PR notes; this file is the curated, human-readable history. .move()` (Chrome 110+) so a cancelled/failed export leaves a clearly-labeled, inspectable partial artifact. Falls back to leaving the plain (non-renamed) file on browsers without `.move()` support, or if the rename itself fails (#105). +- `createApp` built the `app` object with a `doc` field, but every other module + (`explain-graph.js`, `results.js`, `schema-detail.js`, `file-menu.js`, + `shortcuts.js`, `app.js` itself) read `app.document` instead — never + assigned, so `app.document || document` silently always fell back to the + global `document`, harmless today only because the two happened to coincide + in both production and tests. `app` now exposes `document` (not `doc`), and + the fallbacks that were provably unreachable (verified per call site against + `makeApp()` / real callers) were dropped; the fallbacks that are + deliberately null/minimal-`app`-tolerant (`detached-view.js`, + `explain-graph.js`, `schema-detail.js`, and `shortcuts.js` — which has a + dedicated `delete app.document` test) were left untouched. (#106) ## [0.1.5] - 2026-06-29 diff --git a/src/ui/app.js b/src/ui/app.js index 5af77dc..3a1fc6a 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -53,7 +53,7 @@ export function createApp(env = {}) { state: createState(), dom: {}, root: env.root || doc.getElementById('root'), - doc, + document: doc, token: ss.getItem('oauth_id_token'), refreshToken: ss.getItem('oauth_refresh_token'), // Charting seam: the Chart.js constructor (injected so tests stub it) and a @@ -1478,7 +1478,7 @@ export function createApp(env = {}) { /** Build the signed-in shell and mount all regions. */ export function renderApp(app, helpers) { - const { state, doc } = app; + const { state, document: doc } = app; doc.documentElement.setAttribute('data-theme', state.theme); doc.documentElement.setAttribute('data-density', state.density); @@ -1613,11 +1613,11 @@ export function renderApp(app, helpers) { // editor being focused so selecting elsewhere (results, address bar) is ignored. app.syncSelection = () => { const ta = app.dom.editorTextarea; - const focused = ta && (app.document || document).activeElement === ta; + const focused = ta && app.document.activeElement === ta; const sel = focused ? ta.value.slice(ta.selectionStart, ta.selectionEnd) : ''; app.state.hasSelection.value = sel.trim() !== ''; }; - (app.document || document).addEventListener('selectionchange', app.syncSelection); + app.document.addEventListener('selectionchange', app.syncSelection); // Reactive repaint of the schema tree — replaces the scattered renderSchema() // calls: re-runs on schema load, load error, filter text, or expand/collapse. // Registered here (post-mount) so app.dom.schemaList already exists; the effect diff --git a/src/ui/file-menu.js b/src/ui/file-menu.js index a805bf7..9263e6a 100644 --- a/src/ui/file-menu.js +++ b/src/ui/file-menu.js @@ -68,7 +68,7 @@ export function renderLibraryTitle(app) { /** Open the File dropdown anchored under the File button (Esc / outside-click close). */ export function openFileMenu(app) { if (app.dom.fileMenu) return; - const doc = app.document || document; + const doc = app.document; const list = app.state.savedQueries; const close = () => { doc.removeEventListener('keydown', onKey, true); @@ -225,7 +225,7 @@ function confirmNew(app) { } function openConfirm(app, { title, body, confirmLabel, onConfirm }) { - const doc = app.document || document; + const doc = app.document; const close = () => { doc.removeEventListener('keydown', onKey, true); if (app.dom.fileDialog) { app.dom.fileDialog.remove(); app.dom.fileDialog = null; } diff --git a/src/ui/results.js b/src/ui/results.js index bc5ecef..92fa5db 100644 --- a/src/ui/results.js +++ b/src/ui/results.js @@ -331,7 +331,7 @@ function scriptOutcomeCell(app, e) { * primitive is deferred to #60). Escape / backdrop / ✕ closes. Exported for tests. */ export function openRowsViewer(app, entry) { - const doc = app.document || document; + const doc = app.document; let backdrop; let cancelDrawerDrag = () => {}; const onKey = (ev) => { if (ev.key === 'Escape' && isTopDrawer(doc, backdrop)) close(); }; @@ -668,7 +668,7 @@ export function renderTable(app, r) { * Exported for tests. */ export function expandDataPane(app, r) { - const mainDoc = (app && app.document) || document; + const mainDoc = app.document; return openInDetachedTab(app, { title: 'Data', mode: 'grid', @@ -815,7 +815,7 @@ function attachDrawerResize(app, panel, doc) { } export function openCellDetail(app, name, type, value, targetDoc) { - const doc = targetDoc || (app && app.document) || document; + const doc = targetDoc || app.document; const text = value == null ? '' : String(value); let backdrop; let cancelDrawerDrag = () => {}; diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index deebd4c..e31c543 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -142,6 +142,12 @@ describe('createApp basics', () => { expect(createApp(env({ faviconHref: undefined })).faviconHref).toBe('data:image/y;base64,BB'); linkEl.remove(); }); + it('exposes the injected document as app.document, not just the global document', () => { + const customDoc = document.implementation.createHTMLDocument(''); + const app = createApp(env({ document: customDoc, root: customDoc.createElement('div') })); + expect(app.document).toBe(customDoc); + expect(app.document).not.toBe(document); + }); }); describe('renderApp shell', () => {