fix(article): render article deep links server-side, not 404#23
Merged
Conversation
Direct GET /articles/:slug returned an Express "Cannot GET" 404 for every published article (client-side navigation from within the app worked), breaking deep links and hiding article content from crawlers. Root cause was in the SSR bundle, not the route table. markdown.service.ts imported isomorphic-dompurify, which pulls jsdom into the server bundle. jsdom loads its default stylesheet at module-eval time via readFileSync(path.resolve(__dirname, "../../browser/default-stylesheet.css")), and __dirname is undefined in the ESM server bundle -> ReferenceError while evaluating the lazy article-detail chunk -> the router reports a navigation error -> AngularNodeAppEngine.handle() returns null -> Express falls through to its finalhandler 404. It was article-specific because only article-detail imports MarkdownService; topic pages (no MarkdownService) rendered fine, and a missing slug still 302'd because its resolver redirect ran before the throw. parse() already skips DOMPurify on the server (browser re-sanitizes on hydration), so jsdom was never used server-side -- it was pure dead weight that also cannot be esbuild-bundled. Import the browser DOMPurify build (dompurify) instead; isomorphic-dompurify already delegates to it in the browser, so client sanitization is unchanged. Drops the now-unused isomorphic-dompurify and the redundant @types/dompurify (dompurify v3 ships its own types); jsdom stays as the unit-test DOM environment.
🤖 Augment PR SummarySummary: Fixes broken deep-links for published articles by ensuring the article detail route can be server-rendered instead of falling through to an Express 404. Changes:
jsdom module-eval behavior in the ESM server bundle, restoring 200 + full HTML for /articles/:slug.
🤖 Was this summary useful? React with 👍 or 👎 |
| import { marked } from 'marked'; | ||
| import hljs from 'highlight.js'; | ||
| import DOMPurify from 'isomorphic-dompurify'; | ||
| import DOMPurify from 'dompurify'; |
There was a problem hiding this comment.
MarkdownService.parse() still skips DOMPurify during SSR, but this PR makes article pages successfully SSR again—worth double-checking the trust boundary for markdown inputs so the initial server-rendered HTML can’t contain untrusted/scriptable content before client hydration re-sanitizes.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
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
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
Direct
GET /articles/:slugreturned an ExpressCannot GET404 for every published article — deep links were broken and article content was invisible to crawlers. In-app client-side navigation worked, so it only showed up on direct hits / receipts / SEO.Root cause (SSR bundle, not the route table).
articles/:slugwas correctly registered asRenderMode.Server. The failure was inside SSR rendering:markdown.service.tsimportedisomorphic-dompurify, which pullsjsdominto the server bundle.jsdomreads its default stylesheet at module-eval time viareadFileSync(path.resolve(__dirname, "../../browser/default-stylesheet.css")).__dirnameis undefined in the ESM server bundle →ReferenceErrorwhile evaluating the lazyarticle-detailchunk.AngularNodeAppEngine.handle()returnsnull→ Express falls through to its finalhandler 404.Article-specific because only
article-detailimportsMarkdownService; topic pages rendered fine, and a missing slug still302'd (its resolver redirect ran before the throw).Fix.
parse()already skips DOMPurify on the server (the browser re-sanitizes on hydration), sojsdomwas never used server-side — dead weight that also can't be esbuild-bundled. Import the browser DOMPurify build (dompurify) instead;isomorphic-dompurifyalready delegates to it in the browser, so client sanitization is byte-identical. Drops the now-unusedisomorphic-dompurifyand redundant@types/dompurify(dompurify v3 ships its own types).jsdomstays as the unit-test DOM environment.Result: article deep links now true SSR (200 with full body prose, JSON-LD, OG meta,
<title>, canonical) — crawler/SEO-visible, not a shell fallback. No route-table or other-route changes.Test Plan
Reproduced against the production bundle locally (Node fetch rewrite
backend:8080→ live API) — before/after:GET /articles/bounded-autonomy-personal-agent-os: 404 → 200, body carries 9<h2>+ 39<p>of rendered prose, JSON-LDBlogPosting,og:title, SSR<title>+ canonical.…重新理解-go…,…你的變數住在-stack-還是-heap,每一行程式碼…): 404 → 200 with content./,/articles,/topics/:slug→ 200; missing slug → 302 →/not-found.Verification gate (all green):
npx tsc --noEmit— passnpx ng lint— all files passnpx ng test— 617/617 pass (incl.markdown.service.spec.tsXSS sanitization, 24 tests, exercisingdompurify)npx ng build --configuration production— pass; jsdom bundling warning gone