Skip to content

refactor(server): split annotation routes via Bun routes API#170

Merged
blimmer merged 1 commit into
mainfrom
refactor/server-annotation-routes
May 19, 2026
Merged

refactor(server): split annotation routes via Bun routes API#170
blimmer merged 1 commit into
mainfrom
refactor/server-annotation-routes

Conversation

@blimmer
Copy link
Copy Markdown
Contributor

@blimmer blimmer commented May 18, 2026

Summary

The annotation server's fetch handler was a long if/else chain branching on req.method and url.pathname — felt messy and didn't scale to the upcoming review surface. This swaps it for Bun's declarative Bun.serve({ routes }) API and splits each handler into its own file under packages/server/src/routes/, so annotation.ts becomes a thin composition layer (routes table + lifecycle) and new routes drop in beside the existing ones.

Tests follow the split: per-route HTTP integration tests live colocated as routes/<name>.test.ts and share a withServer(ctx, opts?, fn) helper that defaults html/payload/config via new Fishery factories. annotation.test.ts keeps smoke + lifecycle coverage (404 fallback, resolveListenPort, and the existing submit→close race regression).

Review focus

  • /submit invariants — the setTimeout(() => resolveResult(...), 0) deferral and connection: close header are non-negotiable (they prevent Bun from tearing down the socket before the 204 is flushed). They moved into routes/submit.ts verbatim and the race regression test in annotation.test.ts still passes, but worth a second pair of eyes.
  • Test surface tradeoffapp.fetch(req) in-process is gone; everything is real HTTP against a started server now. Slightly slower per test, but a stronger integration boundary and removes the createAnnotationServerApp factory whose only consumer was the tests themselves.
  • withServer overload — opted for two signatures ((ctx, fn) vs (ctx, opts, fn)) over a required {} middle arg. The overload impl is 4 lines of typeof optsOrFn === 'function' — let me know if you'd prefer the simpler always-3-arg shape.

Commits

  • 6754e35 — refactor(server): split annotation routes via Bun routes API

Replace the single-handler if/else chain in `startServer` with Bun's
declarative `Bun.serve({ routes })` API, with each route extracted to
its own file under `packages/server/src/routes/`.

- `annotation.ts` is now a thin composition layer (routes table +
  lifecycle); `createAnnotationServerApp` and `AnnotationServerApp` are
  removed since nothing else imported them.
- Per-route tests live colocated as `routes/<name>.test.ts` and share a
  `withServer(ctx, opts?, fn)` helper that defaults html/payload/config
  via Fishery factories. `annotation.test.ts` keeps smoke + lifecycle
  coverage (404 fallback, resolveListenPort, the submit/close race
  regression).
- Adds `annotationPayload` and `frontendConfig` factories to
  `@contextbridge/shared/testFactories`.

The /submit non-negotiables (`setTimeout` + `connection: close`) are
preserved in the new `handleSubmit`.
@blimmer blimmer marked this pull request as ready for review May 18, 2026 21:38
@blimmer blimmer requested a review from jcarver989 as a code owner May 18, 2026 21:38
Copy link
Copy Markdown
Contributor

@jcarver989 jcarver989 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a nice cleanup here thanks

@blimmer blimmer merged commit 7f62554 into main May 19, 2026
11 checks passed
@blimmer blimmer deleted the refactor/server-annotation-routes branch May 19, 2026 13:36
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.

2 participants