deps(tko.io): use static JSON imports for pkg + test manifest#367
deps(tko.io): use static JSON imports for pkg + test manifest#367brianmhunt merged 1 commit intomainfrom
Conversation
Replaces the process.cwd()-relative readFileSync/Bun.file calls with direct JSON imports. Vite inlines the data at build time, so there is no runtime path resolution and no dependency on cwd being tko.io/. Fixes an adversarial-review concern: process.cwd() assumes the build is invoked from tko.io/ (CI does this, but future scripts calling astro build from repo root would silently produce a missing version badge and empty tests chunk list). Verified: bun run build → 66 pages, TKO v4.0.1 rendered, 32 modulepreload links on /tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughTwo Astro files migrate from runtime filesystem-based JSON reads to static module imports: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tko.io/src/pages/tests.astro`:
- Around line 10-11: The code currently uses "const sourceChunks: string[] =
manifest.chunks ?? []" which silently ignores a malformed manifest; update
tests.astro to validate the manifest shape and fail fast: check that "manifest"
is an object and "Array.isArray(manifest.chunks)" before assigning to
"sourceChunks" (or throw a clear Error/abort rendering if the check fails) so
missing or non-array "chunks" from the imported manifest triggers an explicit
failure instead of falling back to an empty array.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 94cf2f48-19d9-40ae-9406-a2f786c3f0b6
📒 Files selected for processing (2)
tko.io/src/components/Header.astrotko.io/src/pages/tests.astro
| import manifest from '../../public/tests/source/manifest.json' | ||
| const sourceChunks: string[] = manifest.chunks ?? [] |
There was a problem hiding this comment.
Fail fast when the manifest shape is wrong.
manifest.chunks ?? [] silently emits no modulepreload links if a stale/malformed manifest lacks chunks, which defeats the WebKit race mitigation. Since bundle-tests.mjs is expected to write chunks, validate the shape instead of falling back.
Proposed fix
import manifest from '../../public/tests/source/manifest.json'
-const sourceChunks: string[] = manifest.chunks ?? []
+if (!Array.isArray(manifest.chunks)) {
+ throw new Error('tests/source/manifest.json must include a chunks array')
+}
+const sourceChunks: string[] = manifest.chunksBased on learnings, tko.io/src/pages/tests.astro should warm shared ESM chunks using SSR-rendered <link rel="modulepreload"> tags from the manifest chunk list.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import manifest from '../../public/tests/source/manifest.json' | |
| const sourceChunks: string[] = manifest.chunks ?? [] | |
| import manifest from '../../public/tests/source/manifest.json' | |
| if (!Array.isArray(manifest.chunks)) { | |
| throw new Error('tests/source/manifest.json must include a chunks array') | |
| } | |
| const sourceChunks: string[] = manifest.chunks |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tko.io/src/pages/tests.astro` around lines 10 - 11, The code currently uses
"const sourceChunks: string[] = manifest.chunks ?? []" which silently ignores a
malformed manifest; update tests.astro to validate the manifest shape and fail
fast: check that "manifest" is an object and "Array.isArray(manifest.chunks)"
before assigning to "sourceChunks" (or throw a clear Error/abort rendering if
the check fails) so missing or non-array "chunks" from the imported manifest
triggers an explicit failure instead of falling back to an empty array.
There was a problem hiding this comment.
Pull request overview
Updates the tko.io Astro/Starlight site to avoid runtime file reads that depend on process.cwd() by switching to static JSON imports that Vite can inline at build time.
Changes:
- Replace runtime manifest loading in
/testspage with a static JSON import. - Replace runtime
package.jsonreading in the Header with a static JSON import to render the version badge.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tko.io/src/pages/tests.astro |
Imports the generated test manifest JSON directly to inline chunk preload data at build time. |
tko.io/src/components/Header.astro |
Imports the Knockout build package.json directly to inline the version used by the header badge. |
Summary
Follow-up to #360. Replaces the `process.cwd()`-relative `readFileSync` / `Bun.file` calls in `Header.astro` and `tests.astro` with direct static JSON imports.
Vite inlines the data at build time, so there is no runtime path resolution and no dependency on cwd being `tko.io/`.
Why
Adversarial review on #360 flagged that `process.cwd()` assumes `astro build` is invoked from `tko.io/`. CI does that (`cd tko.io && bun run build`), but any future script running from the repo root would silently produce a missing Header version badge and an empty `/tests` chunk list.
Static JSON imports remove the assumption entirely — the build fails loudly at import time if a file is missing, and cwd stops mattering.
Test plan
Summary by CodeRabbit