Skip to content

Render package and browse pages on the server - #164

Merged
mgoldsborough merged 6 commits into
mainfrom
ssr-package-pages
Jul 29, 2026
Merged

Render package and browse pages on the server#164
mgoldsborough merged 6 commits into
mainfrom
ssr-package-pages

Conversation

@mgoldsborough

@mgoldsborough mgoldsborough commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What

React Router framework mode, so the registry's indexable pages render on the server. /packages/* and / get loaders and per-route meta; the sitemap becomes an SSR route; the marketing pages leave for mpak-web; the Playwright prerender and everything supporting it is deleted.

Why

Every /packages/* URL returns byte-identical homepage HTML — same md5 as / — with <link rel="canonical" href="https://www.mpak.dev/">. All 41 tell Google they are the homepage, so none can index. /bundles ships a rendered "Failed to fetch". Both come from HTML assembled at build time by a headless browser that could not reach the API.

Test

CI green. 77 tests (was 60). Verified against the live registry: real per-package title and canonical, SoftwareApplication + BreadcrumbList, 41 packages server-rendered on /, 41 sitemap URLs, ?search= seeded, unknown package → 404, crafted traversal → 404, registry unreachable → / 200 and 503 elsewhere rather than 500.

Deploy coupling

Deploying this is the cutover. It must land with:

  • mpak.dev → GitHub Pages (mpak-web#5) and the app on registry.mpak.dev
  • deployments/mpak/*/web-values.yaml: raise resources.limits.memory above 128Mi (a Node SSR process measures ~111 MiB RSS; sized for nginx it will OOMKill), and drop securityContext.runAsUser: 101 so the image's USER node applies
  • mpak-web to serve or redirect /sitemap.xml and /feed.xml, which are live today

healthCheck.path: /health keeps working — this branch adds the route nginx used to serve.

Every /packages/* URL served byte-identical homepage HTML — the Playwright
postbuild wrote the rendered homepage to dist/index.html and nginx fell every
unmatched route back to it, so all 41 package URLs carried the homepage's
title and a canonical pointing at the homepage. They could not index. /bundles
was worse: it shipped a rendered 'Failed to fetch' because the prerender ran
during docker build with no registry to reach.

React Router framework mode replaces that. Package and browse routes have
loaders, so the name, description, tools, and trust score are in the first
response; per-route meta exports put the title, canonical, and JSON-LD in the
document instead of applying them from an effect a crawler never runs.

The sitemap becomes an SSR route querying the registry, which removes the
build-time fetch that produced an empty file whenever the build host could not
reach the API.

/bundles?search= is now read on first render. The marketing site's hero search
posts there, and the query was being dropped.

The API is same-origin now that the app and registry share a host, so the
browser uses relative paths — no CORS preflight, and self-hosting stops
requiring a second hostname. Server loaders read MPAK_API_URL at request time
rather than from import.meta.env, which Vite bakes in at build.

Marketing pages move out to mpak-web; what remains is the registry. Deleting
them takes prerender.ts, generate-sitemap.ts, generate-feed.ts,
generate-og-image.ts, useSEO.ts, and nginx.conf with them, and drops Playwright
from the image. The runtime is Node rather than nginx.

Component tests get their own vitest config: the React Router plugin injects a
preamble they never receive, which failed four files outright.
apps/web took a direct dependency on isbot for the server-render bot
check, but the lockfile importer entry was never regenerated, so CI
stops at pnpm install --frozen-lockfile.
The lockfile was stale: an unused isbot dependency reddened CI and would have
failed the image build, since both run --frozen-lockfile. Dropped it along with
react-router-dom and @clerk/react-router, neither of which is imported.

/health was served by the nginx config this branch deletes, and nothing
replaced it. The chart's liveness and readiness probes and the ALB health check
all target it, so deploying would have left every pod unready.

fetch rejects rather than resolving when a connection fails, so guarding only
on res.ok made a registry restart a 500 on every server-rendered route.
Requests now go through fetchRegistry/fetchPackage, which treat unreachable as
a failure the caller decides how to degrade: browse renders and lets the client
fetch, package and sitemap answer 503.

Browse's fallback returned [], which is truthy, so the client's retry never
fired and the page stayed permanently empty. It returns undefined now.

The splat param went into the API path unvalidated, so an encoded traversal
reached the registry's list endpoint and rendered a 500 with a stack trace on
the app's main indexable route. Names are checked against @scope/name first.

og:image was set by the deleted index.html and nothing re-emitted it, leaving
twitter:card claiming a large image that did not exist. The nginx security
headers were dropped the same way.

GTM plumbing is gone rather than restored: the loader lived in the deleted
main.tsx and VITE_GTM_ID was never set in any values file or the Makefile, so
nothing was being collected.

Adds tests for the loaders and the name guard — 60 to 77.
The earlier removal updated the lockfile but not package.json — I verified the
parsed dependency object rather than the file, so frozen-lockfile stayed red
for the same reason it was red before.
Removing it was wrong. `react-router build` injects isbot into the generated
server entry and adds it back to package.json — the build prints "adding
isbot@5 to your package.json, you should commit this change" — then runs
pnpm install, which fails under frozen-lockfile. That is what reddened CI in
the first place: the dependency was fine, the lockfile was not.

The lockfile now carries it, so the build finds it already present and stops
rewriting package.json mid-build.
lint:ci runs biome over the whole repo, not just apps/web/src, and the
tsconfig edits this branch made were left unformatted.
@mgoldsborough

Copy link
Copy Markdown
Contributor Author

Adjudication: nine criticals, all reproduced, all fixed — and the one "Delete Instead" I accepted without checking cost two red CI runs

Rejected

Delete Instead 1 — isbot is imported nowhere, delete it rather than regenerate the lockfile · invalid

premise: accepted, then verified the hard way — the absence claim is literally true: nothing in apps/web/src imports isbot. I took it, deleted the dependency, and CI stayed red. The build log says why:

> react-router build
adding `isbot@5` to your package.json, you should commit this change

react-router build injects isbot into the generated server entry when you don't supply your own entry.server, adds it back to package.json mid-build, and then runs pnpm install — which fails under --frozen-lockfile. So the dependency was never the defect; the stale lockfile was, and the fix is exactly the one the finding advised against. isbot restored, lockfile regenerated, build no longer rewrites package.json.

This is the "absence claims are the ones that cost a mechanism" case, and I walked into it: no import ≠ no consumer when a build step generates the importer. Two red runs and three commits to notice.

Accepted — all nine criticals, each reproduced first

Reproduced Fix
1 stale lockfile CI 30430406078 red at install above
2 /health 404 404 against the built server; chart probes + ALB target it routes/health.ts, registered — deliberately does not touch the registry, so a registry blip cannot restart healthy pods
3 outage → 500 everywhere MPAK_API_URL at a closed port: /, sitemap, /packages/* all 500 fetchRegistry/fetchPackage treat a rejected fetch as failure; / 200, others 503
4 browse fallback never recovers [] is truthy, so !initialPackages is false and the retry never fires loader returns undefined
5 unvalidated splat /packages/..%2f..%2fapp%2fpackages → 500 + TypeError in logs @scope/name checked before the fetch → 404, no request made
6 zero tests no .test.* added 60 → 77
7 og:image gone absent, while twitter:card still claimed a large image socialImageMeta() on the indexable routes
8 security headers dropped only cache-control on the response SECURITY_HEADERS spread per route — React Router applies headers per leaf, so there is no global hook to use
9 lint fails 5 biome errors clean

Re-verified after: / 200 with the registry down, sitemap and package 503, traversal 404 with zero TypeErrors, og:image present, all three security headers present, and the core unchanged — Echo | mpak, 41 packages server-rendered.

Delete Instead 2, 3react-router-dom and @clerk/react-router were genuinely unimported and are gone. 4 — GTM plumbing deleted rather than restored: the loader lived in the deleted main.tsx and VITE_GTM_ID is set in no values file and no Makefile, so nothing was ever collected. Not an analytics regression, dead config.

On the two contradicted body claims

Both correct. "Cacheable at the edge" was wrong — there is no CDN in front of registry.mpak.dev, so s-maxage is inert today; the header is right for when one exists but it was not carrying the argument I made with it. "Let the browser retry" described behavior the code did not have, which is finding 4. Body rewritten.

Suggestions

Took 2 (CMD./node_modules/.bin/), 3 (/v0.1 added to the dev proxy), 4 (18 chunks, not 19 — now 20 with the health route, so the count is out of the body entirely). Took 1 into the body's deploy-coupling section: memory limit, runAsUser: 101, and /sitemap.xml + /feed.xml needing a home on mpak-web.

Verification

CI 30432547651: successlint-typecheck and test-typescript both green, 77 tests.

Not covered by CI: the deploy-coupling items are in another repo's values files, and nothing mechanically links them to this merge.

@mgoldsborough

Copy link
Copy Markdown
Contributor Author

QA review round 2 — SHIP THE CORE · 5 new-surface, 1 debris

Built the branch and ran react-router-serve against the live registry. Every core claim holds: 41 packages server-rendered on /, <title>Echo | mpak</title>, canonical registry.mpak.dev/packages/@nimblebraininc/echo, SoftwareApplication + BreadcrumbList, 41 sitemap URLs, ?search= seeded, unknown → 404, traversal → 404 with no TypeError, registry down → / 200 and 503 elsewhere, /health 200, /bundles → 301. Chrome console on / and a package page: zero messages, so hydration is clean.

No critical below is in the core hunk. One is worth fixing before merge; three are deploy blockers filed as issues — and since merging does not deploy this app, they do not block the merge.

Fix before merge

Security headers are absent on every error response, and the per-route spread rests on a false premise.

Round 1's fix was adjudicated with "React Router applies headers per leaf, so there is no global hook to use." That is not right. @react-router/dev/dist/config/defaults/entry.server.node.tsx receives responseHeaders: Headers and builds the final Response from it for every document request, errors included — revealing src/entry.server.tsx is exactly that global hook. It also makes the isbot dependency explicit rather than build-time magic.

Measured on the built server:

Response Security headers
200 /packages/@nimblebraininc/echo 3/3
404 traversal 0/3
404 unknown package 0/3
404 unmatched route 0/3

The enumeration already has a second miss inside this PR: routes/bundles.tsx exports no headers(), so the 301 ships bare. Every future route defaults to unprotected — the denylist shape.

The fix is subtractive: one src/entry.server.tsx calling responseHeaders.set(...), and delete SECURITY_HEADERS from all five route files. Not another guard.

Deploy blockers (filed, not merge blockers)

Delete instead

@playwright/test and the test:e2e / test:e2e:ui scripts — their only consumer was scripts/prerender.ts:14, deleted here. No config, no specs. This contradicts "the Playwright prerender and everything supporting it is deleted."

Suggestion

components/Breadcrumbs.tsx:21-46 injects a second BreadcrumbList after hydration that contradicts the one from route meta (/ → /bundles → /packages/… vs / → /packages/…), and /bundles is now a 301. The parallel injector useSEO.ts was deleted; this one was missed. Drop the JSON-LD useEffect, keep the visual nav.

Body claims: 19 verified, 2 contradicted

Verified by execution, including the two "why" claims — / and /packages/@nimblebraininc/echo share md5 5d58ed2d7408800b177ba6f8e7390e7d with canonical https://www.mpak.dev/, and /bundles/ does ship a rendered "Failed to fetch". Test count is exact: main 60, branch 77, all pass; CI green on run 30432547651. Chart values confirmed at 128Mi and runAsUser: 101.

Contradicted: the Playwright cleanup claim above, and the ~111 MiB figure.

What looks good

PackageLookup making 404-vs-503 fall out of the type rather than a comment; health.ts deliberately not touching the registry; the isbot reversal resolving itself with the mechanism named — the build no longer rewrites package.json. BadgeSection's window.location.origin sits inside the non-default install tab behind import.meta.env.PROD, so it never evaluates during SSR — checked, not a finding. Net −3381 lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

qa-reviewed QA review completed with no critical issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant