Fix broken API samples, bump DuckDB, and harden the page - #3
Open
melonamin wants to merge 3 commits into
Open
Conversation
The advertised code samples used an API the widget does not have. Verified
against the published pondpilot-widget@1.4.0 bundle and upstream docs/API.md:
initQuery / resetQuery -> initQueries / resetQueries (arrays)
widget.addEventListener -> document.addEventListener (the event bubbles)
e.detail.{rows, columns} -> e.detail.{data, elapsed}
window.createDuckDB() -> removed; no such global exists
The wrong option keys were dropped silently by the config normalizer, so a
reader copy-pasting the flagship example got a widget that ignored their setup
SQL and no error to explain it.
Bump @duckdb/duckdb-wasm 1.31.1-dev1.0 -> 1.33.1-dev57.0 (npm latest). That
package's version line is not the engine version: the page claimed "DuckDB
1.31.1", a release that has never existed. `SELECT version()` reports v1.5.4 on
the new bundle, so the cards now say that.
Add a CSP. connect-src must allow extensions.duckdb.org, because reading an
https:// path makes DuckDB auto-load its httpfs extension from there; blocking
it fails the hero query with a wasm trap rather than a network error. Add SRI
hashes to the CDN scripts, and move the inline scripts into assets/js/landing.js
so script-src needs no 'unsafe-inline'.
Derive every version string from the pinned <script src>, which carries the SRI
hash and is therefore the one place a bump has to happen. Pin the widget theme
to dark, since the page has no light mode.
Replace Font Awesome (a 2021 beta, render-blocking, for five icons) with an
inline SVG sprite. Add a skip link, aria-hidden on decorative glyphs, a real
nav list, and a prefers-reduced-motion guard.
Drop 13 unused custom properties, 5 dead classes, three duplicated selectors,
and a six-column grid whose every child spanned all six.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The footer claimed MIT but the repo had no LICENSE. Add it, matching upstream. Add robots.txt and sitemap.xml to go with the canonical, Open Graph, Twitter, and JSON-LD metadata the page now carries. Add four CI jobs, run on push, PR, and weekly (version drift and link rot happen without anyone touching the repo): html-validate structure and accessibility linkinator every outbound link still resolves check-versions.sh version literals agree with the pin; pin is still npm latest check-sri.sh recorded integrity hashes still match what the CDNs serve All four are npx- or shell-based, so the repo stays build-free and has no package.json. Both scripts were tested to fail on injected drift, not just to pass on a clean tree. Building the link check surfaced a pre-existing 404: the nav "Docs" link pointed at pondpilot.io/docs/widget. That is repointed at the GitHub docs tree in the preceding commit, which is what lets this job pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address five review findings, each verified with a negative test: - Guard the DuckDB engine version. check-versions.sh enforced widget version literals but not the data-duckdb-version literals, so DUCKDB_ENGINE_VERSION in landing.js and the HTML could silently drift — the same hardcoded-version bug the page fixes elsewhere. Now asserts the literals equal the const. - Make duckdb-wasm freshness advisory. Its npm `latest` is a -dev line that republishes often, and a bump also needs a manual browser re-check of the engine version, so a hard gate would turn the weekly build red on upstream's cadence rather than on a real problem. Warns instead of failing; the widget freshness gate stays hard. - Document the stamp-before-Prism ordering in landing.js: rewriting the sample's textContent must precede tokenisation, which holds only because both scripts are deferred and this one is first. - check-sri.sh now sends a User-Agent and retries transient fetch failures so a CDN hiccup does not fail the nightly run. - Add <lastmod> to sitemap.xml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
Addressed all five review findings in 43e3d70. Each fix has a negative test proving it fails on the condition it guards, not just that it passes clean.
Not changed, with reasoning:
README's CI table and DuckDB-bump instructions updated to match the new engine-literal guard and advisory behavior. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The code samples this page advertises used an API the widget does not have. Anyone copy-pasting the flagship example got a widget that silently ignored their setup SQL. This fixes that, bumps DuckDB, and adds the guardrails that stop it recurring.
The correctness bugs
Verified against the published
pondpilot-widget@1.4.0bundle and upstreamdocs/API.md, then confirmed live in a browser:index.htmlsaidinitQuery: '…'initQueries: ['…']— array, pluralresetQuery: '…'resetQueries: ['…']— array, pluralwidget.addEventListener(…)document.addEventListenerconst { rows, columns } = e.detaile.detailis{ data, elapsed, widget }window.createDuckDB?.()The wrong option keys were dropped silently by the config normalizer, and the optional chaining on
createDuckDB?.()quietly madeduckdbInstanceundefined. Both failed in the worst possible way: without an error.Running the results event live confirmed the detail keys are exactly
["data","elapsed","widget"].DuckDB
Bumped
@duckdb/duckdb-wasmfrom1.31.1-dev1.0to1.33.1-dev57.0(npmlatest), verified in a browser before and after.That package's version line is not the engine version. The page claimed "DuckDB 1.31.1", a release that has never existed.
SELECT version()reportsv1.4.1on the old bundle andv1.5.4on the new one, so the cards now say 1.5.4. The explicitduckdbVersionoverride was also identical to the widget's own default, which pinned the page to a stale runtime for no benefit.The CSP nearly shipped a dead hero
Adding a CSP broke the hero demo:
RuntimeError: table index is out of bounds. A wasm trap, not a network error, and nosecuritypolicyviolationevent fires because worker violations don't propagate to the document.Bisected across five variants — the plain
SELECTin the theme cards worked, the remote-parquet query didn't, andconnect-src *fixed it. Cause: DuckDB auto-loads itshttpfsextension fromextensions.duckdb.orgthe moment a query reads anhttps://path. That host is now allowlisted, with a comment, because the symptom points nowhere near the cause.Also in here
integrityon all three CDN resources. Inline scripts moved toassets/js/landing.jssoscript-srcneeds no'unsafe-inline'. (Feature card six advertised the widget as "CSP-ready" while the page hosting the claim had neither.)1.4.0appeared 11 times. Every string now derives from the pinned<script src>— the one tag that can't be templated, since it carries the SRI hash. Literals stay in the markup so the page reads correctly without JS; CI asserts they agree.robots.txt,sitemap.xml, and a 1200×630og.pngrendered from the brand assets through headless Chromium so it uses the real Plus Jakarta Sans.aria-hiddenon decorative glyphs, real nav list,prefers-reduced-motionguard.prefers-color-schemewhile the page has no light mode, so light-OS visitors got a light widget inside a dark hero. Pinned to dark.CI
Four jobs on push, PR, and weekly — version drift and link rot happen without anyone touching the repo.
html-validatelinkinatorcheck-versions.shlatestcheck-sri.shintegrityhashes still match what the CDNs serveAll
npx- or shell-based, so the repo stays build-free with nopackage.json. Both scripts were tested against injected drift, not just a clean tree: a stale version literal, a pin behind npm, and a corrupted hash each exit 1.The link check immediately found a pre-existing 404 — the nav "Docs" link pointed at
pondpilot.io/docs/widget. Repointed at the GitHub docs tree, matching the footer. This is the one change outside the reviewed scope, made because CI would otherwise be red on merge.Verification
Driven in headless Chromium against the real page:
midnight-neon,sunset-sorbet,retro-terminal)html-validate,linkinator, version drift, and SRI all greenKnown gaps
og.pngis 410KB. Palette reduction made it larger (dithering the gradient adds noise) and nopngquant/oxipngwas available. Worth a proper optimizer pass.🤖 Generated with Claude Code