fix(seo): buildMediaUrl handles root-relative paths without doubling the API prefix#1167
fix(seo): buildMediaUrl handles root-relative paths without doubling the API prefix#1167abhishekshankar wants to merge 2 commits into
Conversation
…API prefix
`buildMediaUrl(imageRef, siteUrl)` currently handles two cases:
1. Absolute URL → returned as-is
2. Anything else → treated as a bare media_id and built as
`${siteUrl}/_emdash/api/media/file/${imageRef}`
But the CMS SEO panel (and several other code paths) stores
`seo_image` as a root-relative path that ALREADY includes the API
prefix, e.g. `/_emdash/api/media/file/01KS....svg`. The current code
falls into branch 2 and produces:
https://example.com/_emdash/api/media/file//_emdash/api/media/file/01KS.svg
which 404s, breaking <meta property="og:image"> for every post that
used the SEO panel to set an image.
Repro:
import { getSeoMeta } from "emdash/seo";
const meta = getSeoMeta(
{ seo: { image: "/_emdash/api/media/file/01KS.svg" }, data: {} },
{ siteUrl: "https://example.com" },
);
console.log(meta.ogImage);
// before: https://example.com/_emdash/api/media/file//_emdash/api/media/file/01KS.svg
// after: https://example.com/_emdash/api/media/file/01KS.svg
Fix: add a third branch BEFORE the bare-media_id branch that detects
root-relative paths (`imageRef.startsWith("/")`) and just joins with
the host. Tests cover all three branches plus the no-siteUrl case
and the trailing-slash siteUrl case.
Tests live at packages/core/tests/unit/seo/get-seo-meta.test.ts —
lightweight unit tests, no DB setup needed.
|
PR template validation failedPlease fix the following issues by editing your PR description:
See CONTRIBUTING.md for the full contribution policy. |
|
I have read the CLA Document and I hereby sign the CLA Abhishek Shankar seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. |
There was a problem hiding this comment.
This is a correct, well-scoped fix for a real bug.
The root cause is confirmed: SeoImageField.tsx stores local OG images as root-relative paths like /_emdash/api/media/file/{storageKey}. buildMediaUrl previously only distinguished absolute URLs from bare media IDs, so those root-relative paths fell into the bare-ID branch and got the API prefix prepended a second time, producing a 404 URL.
The fix adds an early startsWith("/") branch that joins the path directly with siteUrl (or returns it as-is when no siteUrl is provided). The logic is sound and matches the existing absolutizeMediaUrl helper in page/absolute-url.ts.
The new unit tests in packages/core/tests/unit/seo/get-seo-meta.test.ts cover the regression and relevant edge cases: absolute URLs, root-relative with/without siteUrl, bare media IDs, and trailing-slash stripping. They mirror the source structure and will be picked up by the existing vitest config (tests/**/*.test.ts).
One AGENTS.md convention is missing: this PR changes the published emdash package and therefore needs a changeset in .changeset/.
| } | ||
|
|
||
| // Build from media API path | ||
| // Root-relative path — the CMS SEO panel stores seo_image as |
There was a problem hiding this comment.
[needs fixing] This PR changes the published emdash package but does not include a .changeset/*.md file. AGENTS.md requires a changeset whenever a published package changes.
Please run pnpm changeset and follow the prompts to add a patch-level changeset describing the fix.
Summary
buildMediaUrl(imageRef, siteUrl)inpackages/core/src/seo/index.tscurrently handles two cases:https://...) → returned as-ismedia_idand built as${siteUrl}/_emdash/api/media/file/${imageRef}But the CMS SEO panel stores
seo_imageas a root-relative path that already includes the API prefix, e.g./_emdash/api/media/file/01KS...svg. Branch 2 fires and produces:…which 404s and breaks
<meta property="og:image">for every post that used the SEO panel to set an image.Repro
Fix
Add a third branch BEFORE the bare-media_id branch that detects root-relative inputs (
imageRef.startsWith("/")) and joins them with the host directly — no API-prefix re-prepending.Test plan
New unit tests at
packages/core/tests/unit/seo/get-seo-meta.test.tscover:These don't need DB setup — direct unit tests against
getSeoMeta.Bonus (separate PR if you'd like)
The same logic gets re-implemented by user code that needs to resolve media references outside of
getSeoMeta(avatar URLs, JSON-LDImageObject.url, list-page thumbnails). Those sites currently either reach forimport { buildMediaUrl } from "emdash/seo"(fails — not exported) or re-implement the logic inline and drift.Exporting
buildMediaUrlwould be a one-line addition:Happy to file as a follow-up.
Reference
We currently carry this fix as a
patch-packagepatch in a downstream fork: https://github.com/abhishekshankar/abhishek-shankar.com-emdash/blob/master/patches/emdash%2B0.9.0.patch — happy to delete it once this lands.