fix(build): build the viewer on npm install so a fresh clone can serve#13
Merged
Merged
Conversation
The README quick start (npm install && npx sideshow serve --open) failed from a git clone with `viewer build missing — run npm run build:viewer first`: nothing in the install path built viewer/dist. Only the from- clone path was broken — the published package ships a built viewer via prepack. Chain `npm run build:viewer` onto the existing prepare script. prepare runs on npm install/ci in the repo root only, so registry consumers are untouched, and vite is a devDependency so it is always present when prepare runs. The viewer is already the codebase's one sanctioned build step; this just runs it where the quick start expects it.
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
2 participants
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.
Problem
The README quick start fails from a git clone:
Nothing in the from-clone install path builds
viewer/dist/index.html, which the server reads at boot. The published npm package is unaffected —prepackships a built viewer — so this papercut only hits people trying the repo itself, i.e. exactly the audience the quick start is for.Fix
Chain the viewer build onto the existing
preparescript:prepareruns onnpm install/npm ciin the repo root only, so:npm install+npx sideshow serve --opennow just workspreparedoesn't run when installing the published tarball, andprepackalready shipsviewer/dist/)preparerunsAlternatives considered
serveauto-builds when dist is missing and vite is available — more magic, bigger diff, and pulls build logic into the Node-builtins-only CLI. Rejected.npm run build:viewer) — smallest diff but leaves the papercut and another step to forget. Rejected.Cost
npm cinow also runs a vite build (~0.3s on my machine, single-file viewer). The e2e job double-builds (Playwright global setup builds too) — negligible.npm pack/npm publish:prepareruns beforeprepack, so the viewer builds twice during packing. Verifiednpm packstill produces a correct tarball (17 files, viewer/dist included).Test evidence
Fresh clone in
/tmp, on this branch:npm test,npm run typecheck,npm run lint,npm run format:checkall pass.