fix(examples): vendor the chart library, and show a CSV table#900
Open
mlennie wants to merge 1 commit into
Open
fix(examples): vendor the chart library, and show a CSV table#900mlennie wants to merge 1 commit into
mlennie wants to merge 1 commit into
Conversation
Both example apps loaded Chart.js from cdn.jsdelivr.net. Agent sandboxes and many corporate networks block CDNs, and the failure is easy to miss: the KPI tiles still render because they need no chart library, while every chart and table stays empty. Vendor the library into each package's public/vendor/ and load it with a relative path, so the pages render with no network at all. The vendored file is [email protected]'s dist/chart.umd.js (MIT), identical to what jsDelivr served, minus its trailing sourceMappingURL line since the .map file is not vendored. It is committed once per example because only a package's own public/ directory is web-served and a symlink out of it is refused, and because an example should stay copy-pasteable on its own. Every example read Parquet, so nothing showed that duckdb.table() reads a CSV just as well. An agent working from these has no reason to know it can skip a conversion step it does not need. storefront now also reads data/regions.csv, a 50-row state-to-region lookup joined from order_items through the customer's state, with a sales_by_region view that uses it. It sits alongside the Parquet rather than replacing any of it: the point is that one model reads both. The html-data-apps skill already said to vendor the chart library, and excused the example for loading one from a CDN. The examples now match the skill. Signed-off-by: Monty Lennie <[email protected]>
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.
Both example data apps loaded Chart.js from
cdn.jsdelivr.net. Agent sandboxes and many corporate networks block CDNs, and the failure is easy to miss: the KPI tiles still render, because they need no chart library, while every chart and the data table stay empty. On a network that drops the packets rather than refusing them, which is what a firewall usually does, the page also hangs for the whole connect timeout before it shows you that dead dashboard.Both examples now ship the library at
public/vendor/chart.umd.jsand load it with a relative path, so they render with no network at all.The CSV half
Every example read Parquet, so nothing showed that
duckdb.table()reads a CSV just as well, and an agent working from these has no reason to know it can skip a conversion step it does not need.storefrontnow also readsdata/regions.csv, a 50 row state to region lookup joined fromorder_itemsthrough the customer's state, with asales_by_regionview that uses it. It sits alongside the Parquet rather than replacing any of it: the point is that one model reads both formats in place. It also shows up in the package's data list next to the Parquet files, which is where someone is most likely to notice it.Notes for review
The vendored file is
[email protected]'sdist/chart.umd.js(MIT), identical to what jsDelivr was serving, minus its trailing sourceMappingURL line since the.mapfile is not vendored. Chart.js v4 ships its UMD build already minified, so this is the same 208KB the page always downloaded, just from the package itself now.It is committed once per example rather than shared. Only a package's own
public/directory is web served, and a symlink pointing out of it is refused by design, so a single shared copy is not possible. Two copies also keep each example copy-pasteable on its own, which is how the docs tell you to use them.regions.csvlives indata/, notpublic/. Everything underpublic/is served raw, so a CSV there would be a download that goes around the model's authorize and query controls.The
malloy-html-data-appsskill already told agents to vendor their chart library, and then excused the shipped example for loading one from a CDN. The examples now match the skill, and the same rule is indocs/html-data-apps.md, where a human reader will actually see it. If you copy the vendor script tag into a page in a subdirectory, note the path is relative to the page, so use../vendor/....scripts/generate-example-data.mjswrites the CSV alongside the Parquet, so the data stays deterministic and regenerable. Re-running the generator produces byte identical files.Verified
Both pages render with every off-machine request blocked. The CDN was blocked in the browser's own resolver rather than with a test hook, in both shapes: refused, and blackholed so the connection hangs. The old pages draw zero charts either way, the new ones draw all of them in 18ms.
A fresh clone with
bun run build && bun run startserves all three packages, and clicking from the home page through to the dashboard works with the CDN blocked. Copyingexamples/html-data-appout to its own directory and running it standalone, the way the quick start describes, also still renders offline.Full gate green: typecheck, lint, prettier, 1159 unit and 222 integration tests, and the Playwright suite at 53/53.