diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 6d0004d..1598d55 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -13,6 +13,10 @@ permissions: jobs: release: runs-on: ubuntu-latest + # Only run for stable version tags (vX.Y.Z). Pre-release tags such as + # v1.3.6-rc.1 contain a hyphen and are skipped here — they are published + # as GitHub pre-releases manually with the built .vsix attached. + if: ${{ !contains(github.ref_name, '-') }} steps: - name: Checkout code uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 diff --git a/.gitignore b/.gitignore index 200e073..f7fec62 100644 --- a/.gitignore +++ b/.gitignore @@ -32,3 +32,9 @@ Thumbs.db *.zip tmp/ .worktrees/ + +# Local browser/extension test harness (not part of the repo) +.playwright-mcp/ +.vscode-test-web/ +_repro_*.html +vscode-web-test-db.png diff --git a/CHANGELOG.md b/CHANGELOG.md index 1711693..2532984 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,55 @@ # Changelog +## 1.3.6 + +### Bug Fixes + +- **VS Code for Web (vscode.dev): databases stuck on the loading screen**: Fixed the browser SQLite worker never starting. The worker entry point calls Node's `parentPort.on('message', …)`, but inside a browser Web Worker the global scope only exposes `addEventListener`, not Node's `.on()` — so the worker threw `TypeError: parentPort.on is not a function` before wiring up its message handler, and every database operation silently timed out. A Node-style `parentPort` adapter is now provided for the browser runtime. This affected all database files (`.db`, `.sqlite`, `.gpkg`, …) in the web build; the desktop build and the website demo were unaffected because they use different worker paths (#418). +- **Error Cause Preservation**: The worker now preserves the original error `cause` when re-throwing a database open failure, so the underlying reason is no longer lost (#351). +- **Web Demo Query Parameterization**: Parameterized the filter queries in the web-demo worker, removing string-interpolated SQL in the demo data path (#401). + +### Performance + +- **Batched ALTER TABLE ADD COLUMN**: `undoColumnDrop` batches its `ADD COLUMN` statements into a single call instead of one per column (#405). +- **Batched ALTER TABLE DROP COLUMN**: `deleteColumns` batches its `DROP COLUMN` statements into a single call (#408). +- **Cached JSON Patches**: The `updateCells` JS fallback caches parsed JSON patches across a batch instead of re-parsing per row (#387). +- **updateCellBatch SAVEPOINT Fallback**: The sequential `hostBridge` cell-batch fallback is wrapped in a single `SAVEPOINT` to avoid per-row transaction overhead (#364). +- **Grid Selection Allocation**: Avoided string-key allocations in batch cell/column selection for large selections (#375). +- **Allocation Trimming**: Dropped `Object.keys` allocations in the JSON-merge-patch key iteration (#362) and in serialization, and tidied the `Uint8Array` marker check (#356). + +### Improvements + +- **Browser Worker Bundle Format**: The browser worker bundle (`out/worker-browser.js`) is now emitted as a classic-worker (IIFE) bundle to match how it is loaded (`new Worker(blobUrl)`), removing a latent module-vs-classic mismatch. Added regression tests that fail if the bundle reverts to ESM or if the browser `parentPort` adapter is removed (#418). +- **UI Module Extraction**: Extracted the pure batch-update logic out of `sidebar.js` into a DOM-free, unit-testable module (#416); split `renderSidebar` into helper functions; and modularized the grid render/events and viewer initialization paths. +- **Code Organization**: Extracted file-signature byte arrays into a shared `FILE_SIGNATURES` table (#374), per-format streaming into `getFormatHelper()` (#380), `maskSensitiveData()` into helpers (#398), a `requireEngine()` helper in `createWorkerEndpoint` (#357), and a `DatabaseMethodName` type alias (#386); converted panel-handler arrow-fields to methods (#383). +- **Type Safety**: Tightened the webview message-handler types (`any` → `unknown`) (#377). +- **Webview Hygiene**: Dropped a CSP-blocked inline `onerror` handler from the codicons `` (#353), and removed several unused imports across `edit.js` (#359), `dnd.js` (#354), `blob-inspector.js` (#355), and `main.ts` (#352). + +### Tests + +- Expanded unit coverage: activation/deactivation entrypoint (#406); `disposeAll` null-skip and child `AggregateError` (#403); WASM `establishConnection` failure + worker termination (#402); RPC transfer-fallback warning args (#394); `doTry` null/non-error branches (#391); `deleteColumns` undo-history fetch-error path (#385); delegated worker-endpoint operations after init (#370); comprehensive virtual-filesystem `writeFile` + delete/rename/stat/watch coverage (#365); and consolidated overlapping error-path coverage. + +### CI / Tooling + +- **PR Workflow**: Added a pull-request workflow that builds, typechecks, and tests on every PR, and fixed the latent type errors it surfaced (#414); addressed follow-up review feedback (#417). + +### Documentation + +- Added GitHub community health files (#343); clarified the auto-save failure comment (#358) and rephrased the save-edit comment in `edit.js` (#382). + +### Website + +- Migrated the website to Tailwind CSS v4 (#415). +- Bumped `react`/`react-dom` to 19.2.6 (#344, #347), `@types/node` to 25.9.1 (#348), and `eslint` to 10.4.0 (#349). + +### Dependencies + +**Extension:** + +- @types/node 25.9.0 → 25.9.1 (#345) +- tsx 4.21.0 → 4.22.3 (#346) +- tmp 0.2.5 → 0.2.7 (transitive, npm_and_yarn group) (#381) + ## 1.3.5 ### Security diff --git a/package.json b/package.json index 30f9ecc..1eec4f0 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "name": "sqlite-explorer", "displayName": "SQLite Explorer", "description": "A powerful SQLite database viewer and editor for VS Code", - "version": "1.3.5", + "version": "1.3.6", "publisher": "zknpr", "license": "MIT", "repository": { diff --git a/scripts/build.mjs b/scripts/build.mjs index 1ff1804..d74b613 100644 --- a/scripts/build.mjs +++ b/scripts/build.mjs @@ -178,7 +178,12 @@ const compileBrowserWorker = () => ...baseWorkerConfig, outfile: resolve(outDir, 'worker-browser.js'), platform: 'browser', - format: 'esm', + // IIFE (not ESM): this bundle is loaded as a CLASSIC Web Worker via + // `new Worker(blobUrl)` in workerFactory.ts. An ESM bundle emits a top-level + // `export{...}` which a classic worker cannot parse ("SyntaxError: Unexpected + // token 'export'"), so the worker never boots and VS Code Web hangs on load. + // IIFE also keeps the classic-worker environment sql.js/emscripten expects. + format: 'iife', mainFields: ['browser', 'module', 'main'], external: ['fs/promises', 'path'], define: { diff --git a/src/platform/threadPool.ts b/src/platform/threadPool.ts index 112d3dd..94596a5 100644 --- a/src/platform/threadPool.ts +++ b/src/platform/threadPool.ts @@ -14,47 +14,95 @@ // ============================================================================ /** - * Minimal event receiver interface for message handling. - * Matches the common subset of browser EventTarget and Node.js EventEmitter. + * Node.js-style message port interface. + * Used for communication in Node.js worker_threads. */ -type MessageReceiver = Pick; +interface NodeMessagePort { + postMessage(data: unknown, transfer?: unknown[]): void; + on(event: string, handler: EventListenerOrEventListenerObject, options?: object): void; + off(event: string, handler: EventListenerOrEventListenerObject, options?: object): void; +} /** - * Browser-style message port interface. - * Used for communication in web worker environments. + * Minimal browser worker global scope surface used by the parentPort adapter. + * DedicatedWorkerGlobalScope provides these DOM APIs for communication with the + * spawning context. */ -interface BrowserMessagePort extends MessageReceiver { +interface BrowserParentPortScope { postMessage(data: unknown, transfer?: Transferable[]): void; + addEventListener: EventTarget['addEventListener']; + removeEventListener: EventTarget['removeEventListener']; } /** - * Node.js-style message port interface. - * Used for communication in Node.js worker_threads. + * Create a Node-style parentPort facade for browser workers. + * + * Node's parentPort.on('message', cb) delivers the message payload directly, + * while browser worker "message" listeners receive a MessageEvent whose data + * property contains the payload. The adapter preserves Node-style delivery for + * message events and forwards other events, such as "error", unchanged. */ -interface NodeMessagePort { - postMessage(data: unknown, transfer?: unknown[]): void; - on(event: string, handler: EventListenerOrEventListenerObject, options?: object): void; - off(event: string, handler: EventListenerOrEventListenerObject, options?: object): void; +export function createBrowserParentPort(scope: BrowserParentPortScope): NodeMessagePort { + // Keyed by handler, then by event name. A single handler may be registered for + // more than one event (e.g. the same callback for 'message' and 'error'), so a + // flat handler->wrapped map would let the second registration overwrite the + // first — leaking the earlier DOM listener and making off() remove the wrong + // one. The per-event inner map keeps each (handler, event) wrapper distinct. + const browserListeners = new WeakMap>(); + + return { + postMessage: (data: unknown, transfer?: Transferable[]) => + transfer ? scope.postMessage(data, transfer) : scope.postMessage(data), + on: (event: string, handler: EventListenerOrEventListenerObject) => { + // Node's parentPort.on('message', cb) passes the message DATA directly; the + // DOM 'message' event wraps it in event.data. For other events (e.g. + // 'error') pass the event through so consumers can read `.message`. + const cb = handler as (payload: unknown) => void; + const wrapped: EventListener = (e: Event) => + cb(event === 'message' ? (e as MessageEvent).data : e); + let byEvent = browserListeners.get(handler); + if (!byEvent) { + byEvent = new Map(); + browserListeners.set(handler, byEvent); + } + byEvent.set(event, wrapped); + scope.addEventListener(event, wrapped); + }, + off: (event: string, handler: EventListenerOrEventListenerObject) => { + const byEvent = browserListeners.get(handler); + const wrapped = byEvent?.get(event); + if (wrapped) { + scope.removeEventListener(event, wrapped); + byEvent!.delete(event); + if (byEvent!.size === 0) browserListeners.delete(handler); + } + }, + }; } // ============================================================================ // Runtime Detection and API Export // ============================================================================ -const isBrowserRuntime = import.meta.env?.VSCODE_BROWSER_EXT; - let WorkerImpl: any; let MessageChannelImpl: any; let MessagePortImpl: any; let BroadcastChannelImpl: any; let parentPortImpl: any; -if (isBrowserRuntime) { +if (import.meta.env?.VSCODE_BROWSER_EXT) { WorkerImpl = globalThis.Worker; MessageChannelImpl = globalThis.MessageChannel; MessagePortImpl = globalThis.MessagePort; BroadcastChannelImpl = globalThis.BroadcastChannel; - parentPortImpl = globalThis; + // In a browser Web Worker the global scope (DedicatedWorkerGlobalScope) is the + // channel to the host, but it exposes addEventListener/postMessage — NOT the + // Node worker_threads `.on()` API that the worker entry (databaseWorker.ts) is + // written against. Without this adapter, `parentPort.on('message', ...)` throws + // `TypeError: parentPort.on is not a function`, so the worker never wires up its + // message handler and the host's RPC hangs forever (VS Code Web "stuck loading"). + const workerScope = globalThis as unknown as BrowserParentPortScope; + parentPortImpl = createBrowserParentPort(workerScope); } else { // Node.js environment try { diff --git a/tests/unit/threadPool_browser_parentport.test.ts b/tests/unit/threadPool_browser_parentport.test.ts new file mode 100644 index 0000000..34c4af6 --- /dev/null +++ b/tests/unit/threadPool_browser_parentport.test.ts @@ -0,0 +1,139 @@ +/** + * Regression tests for the browser parentPort adapter. + * + * databaseWorker.ts expects Node's worker_threads parentPort shape, while a + * DedicatedWorkerGlobalScope exposes DOM event APIs. These tests exercise the + * adapter against a fake worker global scope so the browser behavior is covered + * without depending on a real browser. + */ +import { describe, it } from 'node:test'; +import assert from 'node:assert'; +import { createBrowserParentPort } from '../../src/platform/threadPool'; + +describe('createBrowserParentPort', () => { + it('adapts browser worker scope events to Node-style parentPort methods', () => { + const registeredListeners: Array<{ + event: string; + listener: EventListenerOrEventListenerObject; + }> = []; + const removedListeners: Array<{ + event: string; + listener: EventListenerOrEventListenerObject; + }> = []; + const postedMessages: Array<{ data: unknown; transfer?: Transferable[] }> = []; + + const fakeScope = { + postMessage(data: unknown, transfer?: Transferable[]) { + // The fake scope records the exact payload and transfer list forwarded by + // the adapter so the test can verify postMessage does not transform them. + postedMessages.push({ data, transfer }); + }, + addEventListener(event: string, listener: EventListenerOrEventListenerObject) { + // The fake scope stores listeners exactly as registered, allowing the + // test to invoke the wrapped DOM listener with controlled event objects. + registeredListeners.push({ event, listener }); + }, + removeEventListener(event: string, listener: EventListenerOrEventListenerObject) { + // The fake scope records removals so off() can be checked against the + // wrapped listener that on() installed. + removedListeners.push({ event, listener }); + }, + }; + + const parentPort = createBrowserParentPort(fakeScope); + + assert.strictEqual(typeof parentPort.postMessage, 'function'); + assert.strictEqual(typeof parentPort.on, 'function'); + assert.strictEqual(typeof parentPort.off, 'function'); + + const receivedMessages: unknown[] = []; + const messageHandler = (payload: unknown) => { + receivedMessages.push(payload); + }; + + parentPort.on('message', messageHandler); + + assert.strictEqual(registeredListeners.length, 1); + assert.strictEqual(registeredListeners[0].event, 'message'); + + const messageListener = registeredListeners[0].listener as EventListener; + messageListener({ data: 'database-bytes' } as MessageEvent); + + assert.deepStrictEqual(receivedMessages, ['database-bytes']); + + const receivedErrors: unknown[] = []; + const errorHandler = (event: unknown) => { + receivedErrors.push(event); + }; + const errorEvent = new Event('error'); + + parentPort.on('error', errorHandler); + + assert.strictEqual(registeredListeners.length, 2); + assert.strictEqual(registeredListeners[1].event, 'error'); + + const errorListener = registeredListeners[1].listener as EventListener; + errorListener(errorEvent); + + assert.deepStrictEqual(receivedErrors, [errorEvent]); + + const transferBuffer = new ArrayBuffer(8); + parentPort.postMessage({ ready: true }, [transferBuffer]); + + assert.deepStrictEqual(postedMessages, [ + { data: { ready: true }, transfer: [transferBuffer] }, + ]); + + parentPort.off('message', messageHandler); + + assert.strictEqual(removedListeners.length, 1); + assert.strictEqual(removedListeners[0].event, 'message'); + assert.strictEqual(removedListeners[0].listener, registeredListeners[0].listener); + }); + + it('removes the correct wrapped listener when one handler is reused for two events', () => { + // Regression guard: the adapter must key wrapped listeners by (handler, event), + // not by handler alone. With a flat handler->wrapped map, registering the same + // handler for 'message' then 'error' would overwrite the first wrapper, so + // off('message', …) would remove the wrong (error) listener and leak the + // message one. This reuses a single handler across both events and asserts + // each off() removes the exact wrapper that on() installed for that event. + const registeredListeners: Array<{ event: string; listener: EventListener }> = []; + const removedListeners: Array<{ event: string; listener: EventListener }> = []; + + const fakeScope = { + postMessage() {}, + addEventListener(event: string, listener: EventListenerOrEventListenerObject) { + registeredListeners.push({ event, listener: listener as EventListener }); + }, + removeEventListener(event: string, listener: EventListenerOrEventListenerObject) { + removedListeners.push({ event, listener: listener as EventListener }); + }, + }; + + const parentPort = createBrowserParentPort(fakeScope); + + // Same handler reference registered for two distinct events. + const sharedHandler = () => {}; + parentPort.on('message', sharedHandler); + parentPort.on('error', sharedHandler); + + assert.strictEqual(registeredListeners.length, 2); + const messageWrapped = registeredListeners[0].listener; + const errorWrapped = registeredListeners[1].listener; + // Each event must get its OWN wrapper (the bug would reuse/overwrite one). + assert.notStrictEqual(messageWrapped, errorWrapped); + + parentPort.off('message', sharedHandler); + parentPort.off('error', sharedHandler); + + assert.strictEqual(removedListeners.length, 2); + assert.deepStrictEqual( + removedListeners.map(r => r.event), + ['message', 'error'] + ); + // The wrapper removed for each event matches the one registered for it. + assert.strictEqual(removedListeners[0].listener, messageWrapped); + assert.strictEqual(removedListeners[1].listener, errorWrapped); + }); +}); diff --git a/tests/unit/worker_browser_bundle.test.ts b/tests/unit/worker_browser_bundle.test.ts new file mode 100644 index 0000000..bcf77b0 --- /dev/null +++ b/tests/unit/worker_browser_bundle.test.ts @@ -0,0 +1,43 @@ +/** + * Regression tests for the browser worker bundle format. + * + * VS Code Web loads out/worker-browser.js as a classic Web Worker from a blob + * URL. Classic workers parse scripts with the normal script grammar, so this + * test compiles the bundle with node:vm to catch module-only syntax such as a + * top-level export before it can ship. + */ +import { describe, it } from 'node:test'; +import assert from 'node:assert'; +import { existsSync, readFileSync } from 'node:fs'; +import path from 'node:path'; +import vm from 'node:vm'; + +describe('browser worker bundle', () => { + it('parses as a classic worker script', (t) => { + const bundlePath = path.resolve(process.cwd(), 'out/worker-browser.js'); + + // This validates a build artifact, not source. `out/` is gitignored and the + // `npm test` script does not build first, so on a clean checkout the bundle + // may be absent — skip (don't fail the suite) with a clear hint. CI runs + // `node scripts/build.mjs` before tests, so there the assertions always run. + if (!existsSync(bundlePath)) { + t.skip('out/worker-browser.js not built — run `node scripts/build.mjs` first (CI builds before tests).'); + return; + } + + const source = readFileSync(bundlePath, 'utf8'); + + const isIifeBundle = /^(?:"use strict";)?\s*\(\s*(?:\(\)\s*=>|function\s*\()/.test( + source.trimStart() + ); + assert.ok( + isIifeBundle, + 'out/worker-browser.js must be emitted as an IIFE classic-worker bundle' + ); + + assert.doesNotThrow( + () => new vm.Script(source, { filename: bundlePath }), + 'out/worker-browser.js must parse as a classic worker script' + ); + }); +});