tarballs: redesign preview tarballs index page#1911
Merged
Conversation
Rebuild the static index page produced by `tarballs/scripts/pack.ts`: - Featured `workflow` package up top with prominent install command, copy button, and direct tarball download - Top-of-page metadata chips: short SHA (linked to commit), branch, PR number, build timestamp, package count + total size - Collapsible "What is this?" explainer - Package-manager tab toggle (pnpm / npm / yarn / bun) that swaps the install command for every row in place - Live filter input over the rest of the package list (with `/` shortcut) - Per-row install command, copy button, and direct download - Modern dark/light theme with system preference, Geist-inspired styling Also captures tarball size during pack and renders human-readable byte counts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Contributor
Contributor
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
|
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
workflow with 1 step💻 Local Development
workflow with 10 sequential steps💻 Local Development
workflow with 25 sequential steps💻 Local Development
workflow with 50 sequential steps💻 Local Development
Promise.all with 10 concurrent steps💻 Local Development
Promise.all with 25 concurrent steps💻 Local Development
Promise.all with 50 concurrent steps💻 Local Development
Promise.race with 10 concurrent steps💻 Local Development
Promise.race with 25 concurrent steps💻 Local Development
Promise.race with 50 concurrent steps💻 Local Development
workflow with 10 sequential data payload steps (10KB)💻 Local Development
workflow with 25 sequential data payload steps (10KB)💻 Local Development
workflow with 50 sequential data payload steps (10KB)💻 Local Development
workflow with 10 concurrent data payload steps (10KB)💻 Local Development
workflow with 25 concurrent data payload steps (10KB)💻 Local Development
workflow with 50 concurrent data payload steps (10KB)💻 Local Development
Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
stream pipeline with 5 transform steps (1MB)💻 Local Development
10 parallel streams (1MB each)💻 Local Development
fan-out fan-in 10 streams (1MB each)💻 Local Development
SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
❌ Some benchmark jobs failed:
Check the workflow run for details. |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
TooTallNate
approved these changes
May 4, 2026
Member
TooTallNate
left a comment
TooTallNate
left a comment
There was a problem hiding this comment.
The visual design is nice — featured workflow card, package-manager toggle, live filter, copy buttons, geist-inspired theme. Going to approve since the deployment shape doesn't change and this only affects the static index page.
That said: I'd push back on the hand-rolled HTML-in-JS approach for anything beyond a one-page diff. The new file is 857 lines, and the implementation has some smells that follow naturally from the medium:
- The inline
<script>block (~80 lines) isn't TypeScript-checked. It's inside a template literal inwriteIndexHtml, so the compiler treats it as a string. Refactoring (rename adata-attribute, change a property name) won't surface type errors. The TS half references DOM viadata-attributes that have to match by convention. - Two parallel "render the page" concerns — the
pack.tsscript handles both bundling-time logic (scanning packages, rewriting deps, packing tarballs) and presentation logic (HTML composition, CSS, client-side JS for PM toggle/filter/copy). Those were independent before this PR; conflating them makes both harder to evolve. renderFeatured/renderRowand the inlineapplyPm/applyFilterscript must stay in sync on attribute names, class names, and the catalog JSON shape. Right now there's a hand-rolledescapeHtmlhelper and the contract is "data-install-cmdmust equalescapedName." Easy to break in a refactor; no tests catch the drift.- Hand-rolled escape helpers (
escapeHtml) are correct here but would be unnecessary with JSX or a templating library.
A lightweight Vite + React (or Preact for smaller bundle) SPA would clean this up substantially. The shape I'd suggest:
tarballs/
├── package.json # adds vite, react, @vitejs/plugin-react
├── vite.config.ts # static build → public/
├── index.html # entry HTML
├── src/
│ ├── main.tsx # mount React
│ ├── App.tsx # the page (~80 lines instead of ~600 of HTML+CSS+JS)
│ ├── catalog.ts # types for the JSON catalog
│ └── styles.css # the geist-inspired theme
└── scripts/
├── pack.ts # data-only: scans packages, packs tarballs, writes
│ # public/catalog.json with the build context
└── check-tarballs-smoke.mjs
pack.ts becomes ~140 lines (just the bundling logic), vite build produces the static page that fetches catalog.json, the React component reads it on mount and renders. State (active PM, search query) is useState. Build pipeline is pnpm pack → vite build, and Vercel's existing outputDirectory: "public" works unchanged.
DX wins: HMR while iterating on the design, TypeScript checks the JSX and event handlers, can pull in actual geist (or lucide-react) icons via npm instead of inlined SVG, hot-swap component libraries if the design evolves.
If you don't want to take that on now, this is fine as-is — the page works, the design looks good, and the deployment shape is preserved. But suggest filing a follow-up to migrate before adding the next 200 lines of HTML to this file.
Smaller things I noticed
getBuildContextusesprocess.env.VERCEL_GIT_*for SHA/branch/PR. Local builds get only the SHA/branch fallback viagit rev-parse. Worth double-checking the labels on chips degrade gracefully when those env vars are missing — looks like they do (early-return paths withif (ctx.commitUrl)), but a quick localpnpm --filter tarballs buildtest before merge would confirm.- The catalog JSON is embedded via
<script id="catalog" type="application/json">${escapeHtml(JSON.stringify(catalog))}</script>. JSON-inside-HTML-inside-template-literal is two layers of escape (theescapeHtmlhandles the outer, but the JSON itself can contain</script>if a package description has one — unlikely but possible). Safer to useJSON.stringify(catalog).replace(/</g, '\\u003c')for the embedded JSON, which escapes the dangerous bytes without affecting parseability. formatBytesuses base-2 units (KB = 1024) but labels them as'KB'/'MB'which by convention suggest base-10. Minor butKiB/MiBwould be more precise (or use base-10 with1000).applyFilterruns on every keystroke against a re-queryingArray.from(document.querySelectorAll('.pkg-row')). With ~25 packages this is fine; if the catalog grows substantially, debounce or pre-compute.
Test plan note
The PR description has 8 unchecked test-plan items. They're all manual visual checks (page renders, filter works, copy fires, etc.). Since the Vercel preview URL is the canonical way to validate this, they probably won't be checked off in advance — but worth at least running through them on the preview before merge.
TooTallNate
reviewed
May 4, 2026
| Built ${ctx.commitUrl ? `from <a href="${escapeHtml(ctx.commitUrl)}" target="_blank" rel="noopener">${escapeHtml(ctx.shortSha)}</a>` : `from <code>${escapeHtml(ctx.shortSha)}</code>`} · ${escapeHtml(packages.length.toString())} packages totaling ${escapeHtml(formatBytes(totalSize))} | ||
| </footer> | ||
| </div> | ||
| <script id="catalog" type="application/json">${escapeHtml(JSON.stringify(catalog))}</script> |
Member
There was a problem hiding this comment.
Blocking: this line breaks all client-side interactivity.
Verified on the preview deployment with browser automation — every interactive feature is non-functional:
- ❌ Package-manager toggle (clicking npm/yarn/bun does nothing)
- ❌ Live filter (typing in search shows all rows)
- ❌ Copy buttons (no clipboard write, no
Copiedstate) - ❌
/keyboard shortcut (body keeps focus)
Root cause: escapeHtml(JSON.stringify(catalog)) HTML-encodes the JSON. The serialized catalog ends up in the page as:
<script id="catalog" type="application/json">[{"name":"@workflow/ai","escapedName":"workflow-ai"...JSON.parse(textContent) then throws on the very first " because " isn't valid JSON syntax. The IIFE bails on its first line and none of the event listeners get attached. Confirmed live:
> JSON.parse(document.getElementById('catalog').textContent)
Uncaught SyntaxError: Expected property name or '}' in JSON at position 2 (line 1 column 3)<script type="application/json"> content is treated as text by the HTML parser — it does NOT need HTML entity escaping. The only sequence that can break out of it is </script> (or </ in some legacy parsers). The narrowest correct fix is:
<script id="catalog" type="application/json">${
JSON.stringify(catalog).replace(/</g, '\u003c')
}</script>That escapes < bytes to \u003c in the JSON (legal per the spec — JSON allows \uXXXX escapes for any character), so </script> can't appear in the text and the JSON parses cleanly.
This was the third bullet in my "smaller things I noticed" section above — turns out it's not theoretical, it's the live bug breaking everything client-side. Should not merge until the page works in a real browser.
Contributor
Author
There was a problem hiding this comment.
Fixed in 3df48a7. Went with your replace(/</g, '\u003c') over the bot's </-only escape since it also covers a stray < inside, e.g., a package description. Verified locally that the embedded JSON parses cleanly out of the rendered HTML; preview deploy will validate the four interactive features end-to-end.
TooTallNate
requested changes
May 4, 2026
Member
TooTallNate
left a comment
TooTallNate
left a comment
There was a problem hiding this comment.
Updating my prior review to REQUEST_CHANGES. Ran the visual test plan against the preview deployment with browser automation and discovered that all client-side interactivity is broken — see the inline comment on pack.ts:696 for the root cause and fix.
The escapeHtml(JSON.stringify(catalog)) call HTML-encodes every " in the JSON to ", so JSON.parse(textContent) throws on its first character. The IIFE bails immediately and none of the event listeners (PM toggle, search filter, copy buttons, / shortcut) get attached.
Test plan results from the preview at https://workflow-tarballs-git-pgp-tarball-page-improvements.labs.vercel.dev/:
- ✅ Page renders correctly in light mode (visual layout, chips, featured card, package list)
- ✅
workflowpackage appears as the featured card at the top - ✅ SHA / branch / timestamp / package count chips are present
- ❌ Package-manager toggle doesn't update install commands —
aria-selectedstays on pnpm regardless of clicks - ❌ Live filter doesn't filter — typing in the search input shows all rows unchanged
- ❌ Copy buttons don't fire — no
Copiedstate, no clipboard write - ❌
/keyboard shortcut doesn't focus search — body keeps focus - (couldn't test direct download because earlier broken state interfered, but the
<a download>is plain HTML and probably works)
So the page that ships looks correct visually but is functionally a static catalog — the toggle, filter, and copy buttons are all dead UI.
Quick fix at the call site (one line). After fix, all four interactive features should work. Worth re-testing on the next preview deployment.
The other points from my prior review (Vite + React refactor suggestion, the smaller TS-checkable concerns, KB vs KiB nit) all still stand and are non-blocking. Just this one issue blocks merging.
`escapeHtml(JSON.stringify(catalog))` was HTML-encoding every quote in the embedded catalog JSON to `"`, so `JSON.parse(textContent)` threw on the first character and the IIFE bailed before attaching any event listeners — package-manager toggle, search filter, copy buttons, and the `/` shortcut were all dead UI on the deployed page. `<script type="application/json">` content is treated as text by the HTML parser; the only sequence that can break out is `</script>` (or `</` in legacy parsers). Replace `<` with the JSON `<` escape, which is legal per the JSON spec and prevents the breakout without needing entity encoding. Also switch `formatBytes` from `KB`/`MB` to `KiB`/`MiB` since the divisor is 1024. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The smoke check broke in CI: `'workflow' tarball only has 0 files`. Root cause is that `tar -tvzf` emits a different verbose layout on GNU tar (Linux, what CI runs) vs BSD tar (macOS, where I tested locally) — the parser only matched the BSD column ordering, so on Linux every line was rejected and `fileCount` came out as 0. Replace the shell-out with a small in-process tar reader using `zlib.gunzipSync` + manual 512-byte block walk. ustar headers are trivially structured (name at offset 0, octal size at 124, typeflag at 156, ustar prefix at 345). We emit regular files only (`typeflag` `0` or NUL) and consume but skip pax extended headers (`x`/`g`) and GNU long-name entries (`L`). Result is identical on every platform. Verified locally: 206 files / 998413 bytes for `workflow.tgz` matches `tar -tvzf` exactly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous "What's inside?" view crammed nested directory groups, proportional bars, and per-group file lists into a `<details>` inside an already-narrow row. It was hard to read and harder to compare. Replace it with the layout packagephobia uses on its result page: - Two large headline metric tiles (Publish size / Unpacked size) with a big bold value, smaller unit, and small uppercase label. Modeled directly on packagephobia's `Stats` component but using our existing CSS variables so it tracks light/dark theme. - A single sortable file table beneath. Default is size-descending so the contributors to package size are immediately visible. Click a header to flip direction or switch sort key. Sticky header keeps the columns visible inside the scrollable region. Drop the `groupByTopLevel`, `ContentsGroup`, and bar-chart styles — they were the source of the "hard to use" feedback and don't add information that the flat sortable table doesn't already convey. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces the old static tarballs index with a Vite/Preact single-page UI that reads a generated catalog.json, surfaces richer build/package metadata, and adds interactive install/copy/filter/download affordances for preview tarballs.
Changes:
- Adds a Vite + Preact frontend for the tarballs index, including featured package rendering, filtering, package-manager toggles, and package contents breakdowns.
- Extends
scripts/pack.tsto emitcatalog.jsonwith build metadata, tarball sizes, unpacked sizes, and per-file listings. - Updates build/smoke-test plumbing so the tarballs deployment now publishes bundled SPA assets plus the generated catalog.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tarballs/vite.config.ts | Configures Vite to build the SPA into public/ without deleting tarballs. |
| tarballs/turbo.json | Expands build outputs to include SPA assets and catalog.json. |
| tarballs/tsconfig.json | Adds TS config for the new frontend/build files. |
| tarballs/src/styles.css | Adds the full styling/theme for the redesigned index UI. |
| tarballs/src/main.tsx | Boots the SPA and loads catalog.json. |
| tarballs/src/icons.tsx | Adds inline SVG icons used across the new UI. |
| tarballs/src/catalog.ts | Defines shared catalog/install-command helpers for the frontend. |
| tarballs/src/app.tsx | Implements the main tarballs UI, interactions, and package contents tables. |
| tarballs/scripts/pack.ts | Generates tarballs plus the new metadata-rich catalog.json. |
| tarballs/scripts/check-tarballs-smoke.mjs | Adds smoke coverage for catalog.json. |
| tarballs/package.json | Switches build to pack.ts && vite build and adds frontend deps/scripts. |
| tarballs/index.html | Adds the SPA entry HTML shell. |
| pnpm-lock.yaml | Locks newly added frontend/tooling dependencies. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "scripts": { | ||
| "build": "node scripts/pack.ts", | ||
| "build": "node scripts/pack.ts && vite build", | ||
| "dev": "vite", |
Contributor
Author
There was a problem hiding this comment.
Done in 1c48345. The new dev chains node scripts/pack.ts && vite, and I restructured the build layout to vite's conventional shape so it actually works in dev: public/ is a real vite public dir (pack writes tarballs + catalog.json there, vite serves them at root in dev), and dist/ is the production build output (set as Vercel's outputDirectory).
| <SearchIcon /> | ||
| <input | ||
| ref={searchRef} | ||
| type="search" |
Contributor
Author
There was a problem hiding this comment.
Done in 1c48345. Added aria-label="Filter packages" to the input.
Comment on lines
+178
to
+185
| <div class="pm-tabs" role="tablist" aria-label="Package manager"> | ||
| {options.map((opt) => ( | ||
| <button | ||
| key={opt} | ||
| type="button" | ||
| class="pm-tab" | ||
| role="tab" | ||
| aria-selected={value === opt} |
Contributor
Author
There was a problem hiding this comment.
Done in 1c48345. Dropped role="tablist"/role="tab"/aria-selected and made these plain buttons with aria-pressed. Toggle buttons are the honest representation since we never implemented arrow-key roving focus. Each button also has an explicit aria-label (Show install commands for pnpm etc.).
Comment on lines
+428
to
+440
| try { | ||
| await navigator.clipboard.writeText(text); | ||
| } catch { | ||
| const ta = document.createElement('textarea'); | ||
| ta.value = text; | ||
| document.body.appendChild(ta); | ||
| ta.select(); | ||
| try { | ||
| document.execCommand('copy'); | ||
| } finally { | ||
| ta.remove(); | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Done in 1c48345. Refactored to a writeToClipboard helper that returns whether the write actually succeeded; the button only flips to Copied on success. If both navigator.clipboard.writeText and the execCommand fallback fail, it briefly shows a red Failed state instead.
| <code class="pkg-cmd">{cmd}</code> | ||
| <div class="pkg-actions"> | ||
| <CopyButton text={cmd} variant="icon" /> | ||
| <a class="icon-btn" href={pkg.url} download aria-label="Download"> |
Contributor
Author
There was a problem hiding this comment.
Done in 1c48345. Added an accessibleName prop to CopyButton and pass Copy install command for <pkg.name> from each row + the featured card. Same treatment on the download <a> link (Download <pkg.name> tarball).
| if (!root) throw new Error('No #app root element'); | ||
|
|
||
| try { | ||
| const res = await fetch('/catalog.json', { cache: 'no-store' }); |
Contributor
Author
There was a problem hiding this comment.
Done in 1c48345 — dropped cache: 'no-store'. Each preview's URL is unique so the browser cache is the right thing to use.
- main.tsx: drop `cache: 'no-store'` from the catalog fetch. Each tarballs deployment is immutable per commit, so HTTP caching is appropriate; forcing no-store made every visit re-download the full catalog (which now includes per-package file lists). - app.tsx (search input): add `aria-label="Filter packages"`. The visible label only contained an icon and placeholder, so screen readers had no name for the control. - app.tsx (PmTabs): replace `role="tablist"` / `role="tab"` / `aria-selected` with plain buttons that use `aria-pressed`. The ARIA tab pattern requires arrow-key roving focus we never wired up; toggle buttons are the honest representation. Each button also gets an explicit `aria-label`. - app.tsx (row buttons): include the package name in the accessible label of every per-row copy/download button (and on the featured card too), so the screen reader buttons/links list distinguishes them. Added an `accessibleName` prop to `CopyButton`. - app.tsx (CopyButton): only flip to the "Copied" state when the write actually succeeded. Both the modern `navigator.clipboard` path and the `execCommand` fallback can fail; the new `writeToClipboard` helper returns success and the button shows a short "Failed" state if both paths fail. # Make `pnpm dev` work from a clean checkout The previous `dev: vite` couldn't actually serve the page because `/catalog.json` 404s and the SPA boots into the error fallback. Restructure the build layout to vite's conventional shape: - `public/` is now a true vite public dir — pack writes tarballs and catalog.json there. In dev, vite serves these at the root. - `dist/` is the production build output (vite copies public/ into it and adds index.html + assets/). - `vercel.json#outputDirectory` switches from `public` → `dist`. - `turbo.json` outputs updated to match. - `dev` chains pack before vite so the catalog exists when the dev server starts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
TooTallNate
approved these changes
May 4, 2026
Member
TooTallNate
left a comment
TooTallNate
left a comment
There was a problem hiding this comment.
Pranay took the Vite + Preact refactor recommendation from my prior review and ran with it. The new shape matches what I sketched almost exactly:
tarballs/
├── index.html # 20-line entry
├── vite.config.ts # 16-line config
├── tsconfig.json # strict, jsxImportSource: preact
├── scripts/
│ ├── pack.ts # 317 lines, data-only — packs tarballs, writes catalog.json
│ └── check-tarballs-smoke.mjs # now also validates catalog.json shape
└── src/
├── main.tsx # 26 lines — fetch + mount
├── app.tsx # 550 lines — typed Preact JSX
├── catalog.ts # types + small helpers
├── icons.tsx # 8 inline SVG icons
└── styles.css # 794 lines, geist-inspired theme
Bundle ships as dist/assets/index-*.js at 26.25 kB (9.69 kB gzipped) — small for a Preact SPA with sortable tables, copy-buttons, search, and a PM toggle. CSS is 11.19 kB / 2.73 kB gzipped.
What I verified
- Local build works:
pnpm turbo build --filter=tarballsrunspack.tsthenvite build, producing 26 tarballs inpublic/, acatalog.jsonmatching the typed shape insrc/catalog.ts, and adist/ready to deploy. - TypeScript is clean:
pnpm exec tsc --noEmitfromtarballs/passes with the strict config (strict,noUnusedLocals,noUnusedParameters,isolatedModules). - Deployed preview serves correctly:
https://workflow-tarballs-git-pgp-tarball-page-improvements.labs.vercel.dev/catalog.jsonreturns the expected payload with build context (sha, branch, PR number, GitHub-linked commit/branch/PR URLs) and 26 packages with full file listings + sizes. Each package'surlis the immutable per-deployment hash URL (e.g.https://workflow-tarballs-ch8zhxmen.labs.vercel.dev/workflow-ai.tgz), so the catalog is stable even if visited via the git-branch URL. - JS bundle contains real Preact: read the bundle, confirmed the App tree renders with hooks (
useState,useEffect,useMemo,useRef),addEventListener('keydown', …)for the/shortcut,navigator.clipboard.writeText+execCommand('copy')fallback forCopyButton, and thedata-copied/data-failedstate machine. - Smoke check is stronger now: in addition to verifying tarballs are gzip-magic-prefixed and the index page responds with
text/html, it now fetchescatalog.jsonand asserts theworkflowpackage has a non-trivialfileCount— a useful diagnostic for "packages weren't built before pack."
Concerns from my prior review — all addressed
- The hand-rolled HTML-in-JS approach — gone. JSX is TypeScript-checked end to end (event handlers, prop types, catalog shape).
- Two parallel render concerns merged into one file — split clean.
pack.tsis now data-only (~317 lines, all bundling/tar-reading). The presentation layer is insrc/. renderRow/applyPm/applyFilterhad to stay in sync via string conventions — gone. State lives in React-style hooks, props are typed.- Hand-rolled
escapeHtml— gone. JSX handles escaping. escapeHtml(JSON.stringify(catalog))breaking client interactivity — moot. The catalog is a separate/catalog.jsonstatic file fetched at runtime, so there's no inline-JSON encoding question.KB/MBlabels with base-2 math — fixed.formatBytesnow emitsB/KiB/MiB.
Copilot review items — all addressed in 1c48345e6
- a11y:
aria-label="Filter packages"on search input; PM tabs use plain buttons witharia-pressed(no half-implementedrole="tablist"); per-row copy buttons getaccessibleName="Copy install command for ${pkg.name}"instead of a generic label. - Robustness:
writeToClipboardreturns a boolean so failed writes show "Failed" rather than a misleading "Copied" success state. - DX:
devscript chainspack.ts && vite; droppedcache: 'no-store'since the catalog URL is per-deployment-immutable.
Smaller improvements landed along the way
- Tar reader rewrite (
6ae7736): replacedtar -tvzfshell-out with an in-process gunzip + 512-byte block walker. Handles GNU long-name (L) entries and pax headers, and works identically on macOS BSD tar and Linux GNU tar. Sorting files by descending size before emitting is a nice touch for the per-package "what's inside" view. - PackagePhobia-style
SizeStatswidget: two big tiles (publish size + unpacked size) with split value/unit rendering. Looks good and reads better than a one-liner. FileTablewith sortable columns: ARIAaria-sorton<th>, indicator arrows, click-to-toggle direction. Strips thepackage/prefix from displayed paths.
Tiny things — non-blocking
pack.ts:117:updateDepsbuilds rewrite URLs ashttps://${process.env.VERCEL_URL}/${escapedName}.tgzdirectly, whilebaseUrl(defined at line 97) uses theVERCEL_URL ? https://… : ''fallback. Local builds withoutVERCEL_URLproduce package.json deps likehttps://undefined/.... Pre-existing on main, not introduced by this PR. Not a real-world issue (nobody installs from a local pack output) but noting for a follow-up cleanup if you want consistency.catalog.tsandpack.tsduplicate the same TS interfaces (TarballFile,PackedPackage,BuildContext,Catalog). Drift risk is low since they're both in the same package, but if you want to eliminate it,catalog.tscould be the single source of truth andpack.tscouldimport typefrom it. Fine to leave.SizeStatsis rendered for every package in the list, not just the featured one — but it's behind a<details>so it doesn't blow up initial render. The 26 expanded<details>plus the FileTable for each would render a lot of DOM if expanded all at once; in practice, users open one or two. Not blocking.
Verdict: approve
Approving and clearing my prior CHANGES_REQUESTED. The redesign hit every concern from my last two reviews plus the Copilot feedback, the build pipeline is cleaner than what was there before, and the deployment shape is preserved (public/<escaped-name>.tgz URLs unchanged, smoke checks pass). Nice work on the followups.
Contributor
Author
|
@v0 @vercel can you make a PR to address the remaining follow ups from @TooTallNate's review comments that we didn't get to before merge of this one |
2 tasks
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.
Summary
Reworks the static index page generated by
tarballs/scripts/pack.ts(deployed at https://workflow-tarballs.labs.vercel.dev/) to be more useful and pleasant to look at.What's new
workflowpackage at the top with a large install command, copy button, and direct tarball download — the rest of the packages are secondary./keyboard shortcut to focus.Implementation notes
fs.statafterpnpm packso the page can show human-readable sizes.VERCEL_GIT_*env vars for commit / branch / PR links and falls back to localgit rev-parsefor branch when running off-Vercel.workflowis filtered out of the bottom list and rendered separately.public/<escaped-name>.tgzURLs are unchanged, so existing usage keeps working.The Vercel preview for this PR will deploy and serve the new page so you can see it live.
Test plan
workflowpackage appears as the featured card at the top/shortcut)pnpm i https://<deployment>/workflow.tgz)🤖 Generated with Claude Code