Skip to content

Site refresh: writing system, bio rewrite, package upgrades#1

Open
jowch wants to merge 5 commits into
mainfrom
claude/hopeful-hertz-628780
Open

Site refresh: writing system, bio rewrite, package upgrades#1
jowch wants to merge 5 commits into
mainfrom
claude/hopeful-hertz-628780

Conversation

@jowch
Copy link
Copy Markdown
Owner

@jowch jowch commented May 2, 2026

Summary

  • Adds /writing index and /writing/[slug] post detail pages with categorized sections (Essays, Notes, Curiosities & Observations), powered by markdown files in /posts/ with gray-matter frontmatter
  • Rewrites about.md in a more human, precise voice; adds Current Work, Recent Writing, and Projects sections to the homepage
  • Upgrades all packages to latest: Next.js 12→16, React 17→19, Tailwind 3→4 (CSS-first config), FontAwesome 6→7, markdown-to-jsx 7→9, p-throttle 5→8

Notes

  • The placeholder post (posts/on-sequence-space.md) is intentionally excluded from this PR — add real posts before merging or in a follow-up
  • public/feed.xml is committed as a seed; it will be regenerated at each build via the prebuild script in package.json
  • Uses --webpack flag on dev/build due to a Next.js 16 Turbopack incompatibility with Pages Router + fs usage in server modules

Test plan

  • yarn dev — home, /writing, /writing/[slug] all load; dark mode works on all pages
  • Nav links (Writing, home, social icons) all route correctly
  • yarn build succeeds, all static paths resolve
  • /feed.xml is valid RSS (open in browser or feed validator)
  • Mobile viewport: homepage and post detail readable at narrow width

🤖 Generated with Claude Code

- Add /writing index + /writing/[slug] pages with categorized post list
  (Essays, Notes, Curiosities & Observations)
- Add lib/posts.js (gray-matter frontmatter parsing) and lib/rss.js (prebuild RSS feed)
- Rewrite about.md in a more human, precise voice
- Add Current Work, Recent Writing, and Projects sections to homepage
- Add hairline left-rule treatment to post and publication list items
- Upgrade all packages to latest: Next.js 16, React 19, Tailwind 4, FontAwesome 7,
  markdown-to-jsx 9, p-throttle 8
- Migrate to Tailwind 4 CSS-first config (removes tailwind.config.js)
- Fix Next.js 16 Turbopack incompatibility via --webpack flag
- Fix FontAwesome React 19 crash (FA6 → FA7 + react-fontawesome 3.3)
- Fix p-throttle v8 API change in lib/pubs.js
- Fix next/image and next/link API changes for Next.js 13+

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jowch
Copy link
Copy Markdown
Owner Author

jowch commented May 2, 2026

Review of PR #1: Site refresh

Context

PR adds a /writing system (index + post detail), rewrites about.md, adds Current Work / Recent Writing / Projects sections to the homepage, and bumps the toolchain (Next 12→16, React 17→19, Tailwind 3→4, FA 6→7). Net +1073 / −702 across 16 files. The author already flagged that the placeholder post is intentionally excluded.

The review below is grouped by severity. Three of the issues are build-blockers given the current branch state; everything else is cosmetic or a judgment call.

★ Insight ─────────────────────────────────────
Reviewing a PR that simultaneously upgrades a major framework, restructures CSS config, and adds a new feature is hard because failure modes interleave: a build break could be a Tailwind v4 syntax issue, a Next 16 API drop, or the new feature's fault. When you find a problem, locate the layer it lives in before suggesting a fix.
─────────────────────────────────────────────────


🔴 Blockers

1. posts/ directory does not exist on the branch — build will fail

The PR notes say "the placeholder post is intentionally excluded." That left posts/ non-existent on the branch. Three call sites assume it exists:

Result: yarn build fails before any page is generated. (Note that lib/rss.js:11 correctly guards with fs.existsSynclib/posts.js should do the same, or the PR should include at least one post / a .gitkeep.)

Fix: in getAllPosts, return [] when the directory doesn't exist, mirroring the RSS script:

try {
  const files = await fs.readdir(POSTS_DIR)
  // ...
} catch (e) {
  if (e.code === 'ENOENT') return []
  throw e
}

This also makes the recentPosts.length > 0 guard in pages/index.js actually reachable.

2. pages/writing/index.js doesn't serialize Date objects → getStaticProps JSON error

gray-matter parses YAML date: fields into JS Date objects. The other two consumers convert dates to ISO strings before returning (pages/index.js and pages/writing/[slug].js), but pages/writing/index.js:46-49 only strips content:

serialized[cat] = posts.map(({ content: _, ...meta }) => meta)

Once a real post lands, Next will error at build with:

Error serializing .grouped.essay[0].date returned from getStaticProps. Reason: object ("[object Date]") cannot be serialized as JSON.

Fix:

serialized[cat] = posts.map(({ content: _, ...meta }) => ({
  ...meta,
  date: meta.date instanceof Date ? meta.date.toISOString() : meta.date,
}))

Better still: do the conversion once inside parsePost in lib/posts.js so callers don't have to remember.

3. output: 'export' + next start

next.config.js now sets output: 'export', which makes the build emit static HTML and disables next start. The start script in package.json:12 ("start": "next start") will fail. Either remove it or replace with a static server (e.g. npx serve out).

This is minor (you probably don't run yarn start for a GitHub Pages deploy) but worth fixing while the package.json is open.


🟡 Worth fixing before merge

4. Two <h1> per post page

pages/writing/[slug].js:38 renders the title as <h1>, but the Markdown overrides also map any # in the body to <h1> via the Heading component. If a post starts with # Title, the page now has two h1s — bad for SEO and screen readers.

Fix: either map markdown # to h2 (treat the post body as living below the page title), or drop the h1 override entirely and let authors start posts at ##.

5. lib/pubs.jsp-throttle v8 signature change is silently dropped

Old:

const fetchRef = throttle(fn, 200, { leading: true, accumulate: true })

New (lib/pubs.js:13-15):

const fetchRef = throttle(fn)

The 200ms interval, leading, and accumulate options are gone. With p-throttle@8, the API moved to pThrottle({limit, interval})(fn) returning the throttled function — the new code just calls throttle(fn) directly, which (looking at v8's API) treats the whole config object as already applied via pThrottle({limit:2, interval:1000}) higher up. That means throttling is still active (1 req/500ms via limit:2, interval:1000), but the original intent — burst-allow with leading: true and accumulating queued calls — is lost. Worth a comment confirming the new behavior is intentional, or a quick test that hammering CrossRef still doesn't 429.

6. webpack fallback may be a no-op given output: 'export' + Pages Router

next.config.js:8-10 sets config.resolve.fallback = { fs: false, path: false }. With Pages Router + static export, fs/path only run inside getStaticProps/getStaticPaths, which already runs server-side. The fallback is harmless but misleading — it implies a client-side import exists. PR description blames a Turbopack incompat for the --webpack flag, but the fallback hook itself probably isn't necessary. Either drop it or add a comment explaining what triggered it.


🟢 Nits / judgment calls

  • Duplicated PostCard / PostRow between pages/index.js and pages/writing/index.js. Identical markup. Could live in components/post-card.js. Three lines of duplication is fine, but this is closer to fifteen.
  • feed.xml seed contents are stale. The committed seed lists on-sequence-space even though the PR notes say that post is excluded. The prebuild step regenerates, so the seed is only what users see if they git clone and don't build. Consider committing an empty-channel feed or adding a .gitignore line and not committing it at all.
  • Tailwind v4 arbitrary-value syntax max-w-(--breakpoint-md) is correct but unusual — would be worth a one-line note in a CONTRIBUTING-style doc that the project is on Tailwind v4 CSS-first config so future contributors don't reach for tailwind.config.js.
  • escapeXml in lib/rss.js:23 doesn't escape '. Harmless for the current usages (text nodes + double-quoted attributes) but a future contributor putting user data into a single-quoted attribute would be surprised.
  • Image migration in pages/index.js:60: dropped layout='intrinsic' for explicit width/height. Correct for next/image v13+. Good.
  • @heroicons/react removed but I couldn't find any remaining usage in components — confirms the dependency drop is safe.

Verification checklist (when fixing the blockers)

  1. Add a post (or posts/.gitkeep + ENOENT guard in getAllPosts).
  2. yarn install && yarn build — confirm out/ is generated and includes index.html, writing.html, writing/<slug>.html, and feed.xml.
  3. npx serve out — check home, /writing, /writing/[slug] in light + dark mode and at narrow viewport.
  4. Validate out/feed.xml with xmllint --noout out/feed.xml or an online RSS validator.
  5. Confirm pubs.yml resolution still works (the p-throttle v8 change in lib/pubs.js is the riskiest non-build-time path).

Summary

The structural work is solid — the writing system, the lib split, the RSS pipeline, and the v3→v4 Tailwind migration all look right. The merge-blockers are narrow:

  1. Missing posts/ will crash the build (fix in lib/posts.js).
  2. pages/writing/index.js will crash the build once a real post exists (date serialization).
  3. next start is dead with output: 'export'.

Fix those three and the rest can land as follow-up.

jowch and others added 4 commits May 2, 2026 16:58
Blockers:
- lib/posts.js: return [] on ENOENT (missing posts/ dir no longer crashes build)
- pages/writing/index.js: serialize Date fields before returning from getStaticProps
- package.json: replace `next start` with `npx serve out` (output: export drops next start)

Other fixes:
- pages/writing/[slug].js: remap markdown h1 → h2 to avoid duplicate h1 with page title
- components/post-card.js: extract shared PostCard (was duplicated in index + writing)
- next.config.js: add comment explaining why webpack fs/path fallback is necessary
- lib/pubs.js: add comment documenting p-throttle v8 behavior (limit:25/interval:1000)
- lib/rss.js: escape single quotes in escapeXml
- .gitignore: exclude public/feed.xml (regenerated by prebuild script each build)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- lib/posts.js: getPostBySlug returns null on ENOENT (safe for draft/delete workflows)
- pages/writing/[slug].js: return { notFound: true } when post is null
- pages/writing/index.js: category headings h1 → h2 (page has no single h1; multiple h1s broke document outline)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace sm:max-w-fit + sm:justify-between with max-w-(--breakpoint-md) mx-auto,
matching the constraint used on nav, /writing, and /writing/[slug]. Adds pt-[30px]
to match the writing page top padding. Removes float-left from figure (already in
flex container); adds shrink-0 so the headshot width is stable.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Always render border-b-2 with border-transparent so hover doesn't add
height. Applies to both the name link and the icon links.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant