feat: add static site generation mode#147
Open
rohilsurana wants to merge 4 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR implements a complete ChangesStatic Site Mode Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/chronicle/src/components/api/playground-dialog.tsx (1)
220-267:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd missing
useCallbackdependencies to avoid stale request payloads.
handleSendreadsjsonModeandbodyJsonStr, but they are missing from the dependency list, so requests can be sent with outdated body mode/content.Proposed fix
- }, [specName, method, path, serverUrl, pathValues, queryValues, getAuthHeaders, headerValues, bodyValues, body]) + }, [specName, method, path, serverUrl, pathValues, queryValues, getAuthHeaders, headerValues, bodyValues, body, jsonMode, bodyJsonStr])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/chronicle/src/components/api/playground-dialog.tsx` around lines 220 - 267, The handleSend useCallback captures jsonMode and bodyJsonStr but they are not listed in the dependency array, causing stale request bodies; update the dependency array on the useCallback that defines handleSend (the function named handleSend) to include jsonMode and bodyJsonStr so the callback is recreated when either the JSON mode or the JSON string changes.
🧹 Nitpick comments (2)
packages/chronicle/tests/e2e/compare-static-ssr.e2e.ts (1)
26-30: ⚡ Quick winEnsure contexts always close with
try/finallyIf navigation/evaluate/screenshot fails,
ctxSSR.close()andctxStatic.close()may be skipped, which can leak resources across tests.Suggested refactor
test(`compare: ${pg.name}`, async ({ browser }) => { const ctxSSR = await browser.newContext(); const ctxStatic = await browser.newContext(); - const pageSSR = await ctxSSR.newPage(); - const pageStatic = await ctxStatic.newPage(); + try { + const pageSSR = await ctxSSR.newPage(); + const pageStatic = await ctxStatic.newPage(); @@ - await ctxSSR.close(); - await ctxStatic.close(); + } finally { + await ctxSSR.close(); + await ctxStatic.close(); + } @@ });Also applies to: 114-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/chronicle/tests/e2e/compare-static-ssr.e2e.ts` around lines 26 - 30, The test opens Playwright contexts/pages (ctxSSR, ctxStatic, pageSSR, pageStatic) without guaranteeing cleanup; wrap the navigation/evaluate/screenshot logic in try/finally blocks so both contexts are always closed (and pages closed) even on failure — e.g., create ctxSSR/ctxStatic and pages, then use try { ... } finally { await pageSSR?.close(); await pageStatic?.close(); await ctxSSR?.close(); await ctxStatic?.close(); } to ensure no resource leak; apply same pattern to the other occurrence around the lines referencing ctxSSR/ctxStatic (and pages) at 114-115.packages/chronicle/src/server/entry-static.tsx (1)
16-16: ⚡ Quick winUse the configured
@/*alias for internal imports.Line 16 should use the project alias instead of a relative path to stay consistent with repo conventions.
Proposed change
-import { App } from './App'; +import { App } from '`@/server/App`';As per coding guidelines
**/*.{ts,tsx}: Use path alias@/*→./src/*configured in tsconfig and vite.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/chronicle/src/server/entry-static.tsx` at line 16, Import in entry-static.tsx is using a relative path; change the import of App to use the project path alias instead (replace "import { App } from './App';" with the alias form e.g. "import { App } from '`@/server/App`' or '`@/App`' as appropriate"). Update the import statement referencing the App symbol so it resolves via the configured `@/`* alias consistent with tsconfig/vite settings and repo conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/chronicle/src/cli/commands/build.ts`:
- Around line 42-43: The code currently hardcodes outputDir =
path.resolve(projectRoot, '.output/public'), which breaks non-default static
presets; update the build command to derive the output path from the active
preset/config instead of a fixed string: locate where the preset or
static-capable preset is selected (references to outputDir and projectRoot in
build.ts) and read the preset's configured output root (e.g., a property or
method on the preset like outputRoot, outputDir, getOutputDir(projectRoot) or
similar config in the preset object) and resolve that against projectRoot; fall
back to the existing '.output/public' only if the preset provides no output
path. Ensure the variable name stays outputDir so downstream code remains
unchanged.
In `@packages/chronicle/src/cli/commands/static-generate.ts`:
- Around line 544-546: The code uses page.frontmatter.lastModified directly with
toISOString(), which throws for invalid dates; fix by creating a Date from
page.frontmatter.lastModified (e.g., const d = new
Date(page.frontmatter.lastModified)), check its validity via
isFinite(d.getTime()) or !isNaN(d.getTime()), and only produce the lastmod
string when the date is valid; otherwise leave lastmod as an empty string —
update the lastmod construction in static-generate.ts to use this validated Date
logic.
- Around line 619-623: The remote font fetch call (the fetch that assigns to
response and fontData in static-generate.ts) has no timeout and can hang builds;
wrap the fetch in an AbortController with a short timeout (e.g., 5–10s) so the
request is aborted if it exceeds the limit, clear the timer on success, and
handle the abort error in the existing catch to fall back gracefully (e.g., use
a local font or skip). Specifically, modify the block where response = await
fetch(...) and fontData = await response.arrayBuffer() to create an
AbortController, pass controller.signal to fetch, set a setTimeout that calls
controller.abort(), and ensure the timeout is cleared after response is
received.
- Around line 804-860: The code currently allows an empty entryJs when
readViteManifest() returns falsy or no entry with isEntry is found, producing a
broken index.html; after the manifest-parsing loop in static-generate.ts check
for manifest existence and that entryJs is non-empty and if not either throw a
descriptive Error (e.g. "Vite manifest missing or no entry chunk found") or call
process.exit(1) after logging the manifest/manifest keys; update the logic
around readViteManifest, manifest and entryJs to fail fast and surface the
underlying manifest contents to aid debugging.
- Around line 696-716: The code resolves image sources using relativePath from
markdown and directly calls path.resolve(contentDir, relativePath), which allows
path traversal; update the image handling in the loop that uses variables
imgUrl, relativePath, srcPath (and the isSvg branch) to validate the resolved
path: after computing srcPath = path.resolve(contentDir, relativePath) compute a
normalized relative = path.relative(contentDir, srcPath) and skip (or log and
continue) if that relative startsWith('..') or path.isAbsolute(relative)
indicates srcPath is outside contentDir; apply the same guard before copying
SVGs and before converting rasters to webp so any ../ escapes are rejected.
In `@packages/chronicle/src/components/ui/search.tsx`:
- Around line 119-134: The current filters (in the docs branch and the results
branch around ms.search) use startsWith which wrongly matches tags like "v1" to
"/v10/..."; change the check to ensure the tag matches a full path segment by
using a boundary-aware test (e.g., replace the startsWith logic with a regex
like new RegExp(`^/${tag}(?:/|$)`) or compare the first path segment via
url.split('/')[1] === tag); update the filtering logic in both places (the docs
filter that uses docs.filter and the results.filter after ms.search) to use this
boundary-aware check (reference variables/functions: docs, results, ms.search,
tag, query).
In `@packages/chronicle/tests/e2e/check-broken-assets.e2e.ts`:
- Around line 30-31: Remove the swallow of navigation errors so broken assets
fail the test: stop catching errors from page.goto (reference: page.goto) and
let the navigation throw on failure or explicitly rethrow the caught error; then
after the asset-check/report step (the code that writes the report referenced
around lines 48-55), add an assertion that the reported failures count is zero
(e.g., assert failuresCount === 0 or expect(failuresCount).toBe(0)) so the test
fails when any broken assets are found. Ensure page.waitForTimeout remains only
for pacing, not masking navigation errors.
- Around line 6-8: Replace the machine-specific absolute pagesDir and the
import-time fs.readdirSync call: stop hardcoding pagesDir and defer reading
files until test runtime. Construct pagesDir using a project-relative resolution
(e.g., path.resolve(process.cwd(), '…/public/data/pages') or __dirname-based
path) and move the readdirSync logic out of module top-level into the test setup
or a helper function so pageFiles is computed at runtime (refer to pagesDir and
pageFiles to locate and change the code).
In `@packages/chronicle/tests/e2e/compare-static-ssr.e2e.ts`:
- Around line 117-119: The test currently only checks ssrSnap and staticSnap are
not null; update it to actually compare their contents by replacing the two
non-null assertions with a structural/content assertion: e.g., normalize both
snapshots (trim, collapse whitespace, or parse into DOM via cheerio) and assert
equality with expect(ssrSnapNormalized).toEqual(staticSnapNormalized) or, if you
prefer tolerance, compute a diff and assert it falls within acceptable bounds.
Locate the variables ssrSnap and staticSnap in the compare-static-ssr.e2e.ts
test and perform normalization/parsing before the final expect so the test fails
on real content drift.
In `@packages/chronicle/tests/e2e/static-site.e2e.ts`:
- Around line 34-40: Replace the conditional visibility guards with explicit
assertions so tests fail when elements are missing: instead of wrapping the
navigation/assertions in if (await installationLink.isVisible()) { ... }, call
an explicit visibility assertion (e.g., await
expect(installationLink).toBeVisible()) before clicking, then perform
installationLink.click(), await page.waitForTimeout(...), assert URL with
expect(page.url()).toContain('/docs/guides/installation') and check
page.textContent('body') contains 'Installation'; apply the same change to the
second block (the block around lines 52-57) so its link is asserted visible
before interacting.
- Around line 43-49: The test "search dialog opens with Ctrl+K" currently calls
page.keyboard.press('Meta+k') which is macOS-specific; change it to send a
platform-aware shortcut by detecting the OS (e.g., process.platform === 'darwin'
? 'Meta+k' : 'Control+k') before calling page.keyboard.press so the test uses
Meta+k on mac and Control+k on other systems; update the call site where
page.keyboard.press is invoked in this test to use the computed modifier string.
---
Outside diff comments:
In `@packages/chronicle/src/components/api/playground-dialog.tsx`:
- Around line 220-267: The handleSend useCallback captures jsonMode and
bodyJsonStr but they are not listed in the dependency array, causing stale
request bodies; update the dependency array on the useCallback that defines
handleSend (the function named handleSend) to include jsonMode and bodyJsonStr
so the callback is recreated when either the JSON mode or the JSON string
changes.
---
Nitpick comments:
In `@packages/chronicle/src/server/entry-static.tsx`:
- Line 16: Import in entry-static.tsx is using a relative path; change the
import of App to use the project path alias instead (replace "import { App }
from './App';" with the alias form e.g. "import { App } from '`@/server/App`' or
'`@/App`' as appropriate"). Update the import statement referencing the App symbol
so it resolves via the configured `@/`* alias consistent with tsconfig/vite
settings and repo conventions.
In `@packages/chronicle/tests/e2e/compare-static-ssr.e2e.ts`:
- Around line 26-30: The test opens Playwright contexts/pages (ctxSSR,
ctxStatic, pageSSR, pageStatic) without guaranteeing cleanup; wrap the
navigation/evaluate/screenshot logic in try/finally blocks so both contexts are
always closed (and pages closed) even on failure — e.g., create ctxSSR/ctxStatic
and pages, then use try { ... } finally { await pageSSR?.close(); await
pageStatic?.close(); await ctxSSR?.close(); await ctxStatic?.close(); } to
ensure no resource leak; apply same pattern to the other occurrence around the
lines referencing ctxSSR/ctxStatic (and pages) at 114-115.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9cd0e4ce-24d6-4fe9-a541-b831bd2e48e1
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
docs/superpowers/specs/2026-06-03-static-site-mode-design.mdpackages/chronicle/package.jsonpackages/chronicle/playwright.config.tspackages/chronicle/src/cli/commands/build.tspackages/chronicle/src/cli/commands/static-generate.tspackages/chronicle/src/components/api/playground-dialog.tsxpackages/chronicle/src/components/ui/search.tsxpackages/chronicle/src/lib/openapi.tspackages/chronicle/src/lib/page-context.tsxpackages/chronicle/src/lib/preload.tspackages/chronicle/src/server/entry-static.tsxpackages/chronicle/src/server/vite-config.tspackages/chronicle/tests/e2e/check-broken-assets.e2e.tspackages/chronicle/tests/e2e/compare-static-ssr.e2e.tspackages/chronicle/tests/e2e/static-site.e2e.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/chronicle/tests/e2e/check-broken-assets.e2e.ts (1)
37-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNavigation errors are silently swallowed.
The
.catch(() => {})suppresses all navigation failures, so if a page returns 404 or times out, the test proceeds as if the page loaded successfully. This may hide legitimate static site generation issues.If intentional (to audit all pages even when some fail), consider at minimum logging the error:
- await page.goto(`${STATIC_URL}${pageUrl}`, { waitUntil: 'networkidle', timeout: 15000 }).catch(() => {}); + await page.goto(`${STATIC_URL}${pageUrl}`, { waitUntil: 'networkidle', timeout: 15000 }).catch(e => { + failedRequests.push({ pageUrl, resource: `NAVIGATION_FAILED: ${e.message}` }); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/chronicle/tests/e2e/check-broken-assets.e2e.ts` at line 37, The navigation call in the test uses page.goto(`${STATIC_URL}${pageUrl}`, { waitUntil: 'networkidle', timeout: 15000 }).catch(() => {}), which silently swallows navigation failures; change this to surface errors instead — either remove the .catch so failures fail the test, or replace it with a handler that logs the thrown error (including pageUrl and error.message) and rethrows or records a test assertion failure; update the call site in the check-broken-assets.e2e.ts test (the page.goto invocation) so navigation errors are not ignored.
♻️ Duplicate comments (1)
packages/chronicle/tests/e2e/check-broken-assets.e2e.ts (1)
59-59:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNo assertion on
failedRequests— network failures pass silently.The test collects
failedRequestsand writes them to the report, but never asserts on them. A page with broken JS/CSS assets will pass as long as images render. Consider adding an assertion:expect(broken, `Broken images found on ${broken.length} pages`).toHaveLength(0); + expect(failedRequests, `Failed requests on ${failedRequests.length} resources`).toHaveLength(0);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/chronicle/tests/e2e/check-broken-assets.e2e.ts` at line 59, The test currently only asserts on the `broken` array and ignores `failedRequests`, so network failures can pass silently; add an assertion that `failedRequests` is empty (e.g., `expect(failedRequests, \`Network/asset request failures found on ${failedRequests.length} pages\`).toHaveLength(0)`) right after the existing `expect(broken...)` line so any failed JS/CSS/image/network requests fail the test and include a clear message with `failedRequests.length`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/chronicle/tests/e2e/check-broken-assets.e2e.ts`:
- Line 37: The navigation call in the test uses
page.goto(`${STATIC_URL}${pageUrl}`, { waitUntil: 'networkidle', timeout: 15000
}).catch(() => {}), which silently swallows navigation failures; change this to
surface errors instead — either remove the .catch so failures fail the test, or
replace it with a handler that logs the thrown error (including pageUrl and
error.message) and rethrows or records a test assertion failure; update the call
site in the check-broken-assets.e2e.ts test (the page.goto invocation) so
navigation errors are not ignored.
---
Duplicate comments:
In `@packages/chronicle/tests/e2e/check-broken-assets.e2e.ts`:
- Line 59: The test currently only asserts on the `broken` array and ignores
`failedRequests`, so network failures can pass silently; add an assertion that
`failedRequests` is empty (e.g., `expect(failedRequests, \`Network/asset request
failures found on ${failedRequests.length} pages\`).toHaveLength(0)`) right
after the existing `expect(broken...)` line so any failed JS/CSS/image/network
requests fail the test and include a clear message with `failedRequests.length`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3062a2f5-7326-4b29-be9f-690070a74fdb
📒 Files selected for processing (4)
packages/chronicle/src/cli/commands/static-generate.tspackages/chronicle/src/components/ui/search.tsxpackages/chronicle/tests/e2e/check-broken-assets.e2e.tspackages/chronicle/tests/e2e/static-site.e2e.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/chronicle/src/components/ui/search.tsx
- packages/chronicle/tests/e2e/static-site.e2e.ts
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
1 participant
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
--preset statictochronicle buildthat produces a fully self-contained SPA deployable to any static host (GitHub Pages, S3, Netlify, Cloudflare Pages) with no server, no SSR, no databasemeta.jsonfiles for folder titles and ordering in the sidebar treeindex_pageconfig) and env var substitution (${API_SERVER_URL}) gracefullyUsage
Output structure
Files changed
static-generate.tsentry-static.tsxcreateRoot(no hydration), fetches from static JSONbuild.tsvite.build()then runs static generatorvite-config.tspage-context.tsx/data/paths in static modepreload.tssearch.tsxplayground-dialog.tsxopenapi.tsresolveDocumentfor static generator's $ref resolutionTest plan