Skip to content

Fix Markdown content negotiation#528

Open
lucperkins wants to merge 9 commits into
mainfrom
fix-markdown-negotiation
Open

Fix Markdown content negotiation#528
lucperkins wants to merge 9 commits into
mainfrom
fix-markdown-negotiation

Conversation

@lucperkins
Copy link
Copy Markdown
Member

@lucperkins lucperkins commented May 5, 2026

Summary by CodeRabbit

  • New Feature

    • Optional markdown negotiation: pages can emit an alternate link to a markdown version when enabled.
  • Enhancement

    • Page metadata descriptions are now normalized to plain text for SEO and social previews.
    • Layouts and pages can opt in to markdown negotiation so alternate-markdown links appear for enabled pages.
  • Refactor

    • Slug handling for route resolution updated to a revised derivation approach.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 5, 2026

Deploy Preview for zero-to-nix ready!

Name Link
🔨 Latest commit cf8b92d
🔍 Latest deploy log https://app.netlify.com/projects/zero-to-nix/deploys/69fbc178e4292100084d3599
😎 Deploy Preview https://deploy-preview-528--zero-to-nix.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 5, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Start-route slug derivation now uses entry.slug.substring(1) (removing stripExt(entry.id) usage). Added markdown-negotiation plumbing: plainText() util (remark + strip-markdown), deps, a markdownNegotiation prop flowed pages → Layout → Head, Head uses plain text for SEO and can emit a .md alternate link.

Changes

Start-route slug refactor

Layer / File(s) Summary
Data Shape / Helper Removal
src/pages/start/[slug].md.ts
Removed stripExt-based slug derivation; rely on entry.slug instead.
Static Paths Generation
src/pages/start/[slug].md.ts
getStaticPaths sets params.slug from entry.slug.substring(1) (replacing stripExt(entry.id)).
Entry Resolution
src/pages/start/[slug].md.ts
GET handler locates entries by comparing e.slug.substring(1) to params.slug (replacing prior stripExt(e.id) comparison).

Markdown negotiation, plain-text SEO, and deps

Layer / File(s) Summary
New Utility / Dependency
src/lib/utils.ts, package.json
Added export const plainText = async (md: string): Promise<string> using remark().use(strip); added remark and strip-markdown to package.json.
Component API / Types
src/layouts/Layout.astro, src/components/Head.astro
Added optional markdownNegotiation?: boolean to Layout props and destructuring (default false); Head.astro accepts markdownNegotiation.
Core Rendering / SEO
src/components/Head.astro
Imports plainText; computes plainDescription from plainText(description) or siteDescription; uses plainDescription for SEO and twitter.description; conditionally emits a markdown alternate link when markdownNegotiation is truthy.
Wiring / Pages
src/pages/concepts/[slug].astro, src/pages/start/[slug].astro, src/layouts/Layout.astro
Pages updated to pass markdownNegotiation into <Layout />; Layout forwards markdownNegotiation to Head.

Sequence Diagram

sequenceDiagram
  autonumber
  actor Page
  participant Layout
  participant Head
  participant Utils

  Page->>Layout: render(title, description, markdownNegotiation)
  Layout->>Head: render(title, description, markdownNegotiation)
  Head->>Utils: plainText(description) (async)
  Utils-->>Head: plainDescription
  Head->>Head: set SEO metadata (description/twitter) using plainDescription
  alt markdownNegotiation true
    Head->>Head: emit <link rel="alternate" type="text/markdown" href="{canonical}.md">
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • grahamc

Poem

🐰 I nibble bytes beneath the moonlit glade,
Slugs reformed where old helpers used to fade,
Plain-text hums tidy for SEO's ear,
A dot‑md link appears when negotiation's near,
Hopping off, delighted — code polished and made.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main purpose of the changeset—implementing markdown content negotiation support across multiple components and utilities.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-markdown-negotiation

Comment @coderabbitai help to get the list of available commands and usage tips.

@lucperkins lucperkins enabled auto-merge May 5, 2026 15:40
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/Head.astro`:
- Around line 80-83: The alternate markdown link currently uses
`${canonical.href}.md` which can produce a double-slash if canonical.href ends
with a trailing slash; update the Head.astro logic that renders the alternate
link (the markdownNegotiation conditional) to derive the markdown URL from
canonical.pathname with trailing slash removed (e.g., use
canonical.pathname.replace(/\/$/, "") and then append ".md") so the generated
href is normalized and safe regardless of trailingSlash config or atypical
deployments.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8a006a81-08a4-422b-a16d-3074fad5a76a

📥 Commits

Reviewing files that changed from the base of the PR and between 886a6fd and ec6eed7.

📒 Files selected for processing (4)
  • src/components/Head.astro
  • src/layouts/Layout.astro
  • src/pages/concepts/[slug].astro
  • src/pages/start/[slug].astro

Comment thread src/components/Head.astro Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/Head.astro`:
- Around line 80-83: The alternate link object in Head.astro (the entry with
rel: "alternate" and href:
`${canonical.origin}${canonical.pathname.replace(/\/$/, "")}.md`) needs a MIME
type for markdown; add a property `type: "text/markdown"` to that link object so
the alternate-representation discovery explicitly declares the content type.
- Around line 22-24: plainDescription can become an empty string after await
plainText(description) and trim(), which will produce a blank meta description;
change the logic around plainDescription (the const declaration that uses
description, plainText, and siteDescription) to check the trimmed result and if
it's falsy/empty fallback to siteDescription (i.e., compute const stripped =
description ? (await plainText(description)).trim() : ""; then set const
plainDescription = stripped || siteDescription) so empty markdown-stripped
descriptions use siteDescription instead of emitting a blank.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3148074c-c137-46d3-91f5-815a56bcb057

📥 Commits

Reviewing files that changed from the base of the PR and between 2196157 and 20dd8ae.

📒 Files selected for processing (1)
  • src/components/Head.astro

Comment thread src/components/Head.astro
Comment on lines +22 to +24
const plainDescription = description
? (await plainText(description)).trim()
: siteDescription;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fallback if stripped markdown description is empty.

After markdown removal and trim, the result can be empty; this would emit a blank meta description. Please fallback to siteDescription in that case.

Proposed fix
-const plainDescription = description
-  ? (await plainText(description)).trim()
-  : siteDescription;
+const plainDescription = description
+  ? (await plainText(description)).trim() || siteDescription
+  : siteDescription;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Head.astro` around lines 22 - 24, plainDescription can become
an empty string after await plainText(description) and trim(), which will
produce a blank meta description; change the logic around plainDescription (the
const declaration that uses description, plainText, and siteDescription) to
check the trimmed result and if it's falsy/empty fallback to siteDescription
(i.e., compute const stripped = description ? (await
plainText(description)).trim() : ""; then set const plainDescription = stripped
|| siteDescription) so empty markdown-stripped descriptions use siteDescription
instead of emitting a blank.

Comment thread src/components/Head.astro
Comment on lines +80 to +83
{
rel: "alternate",
href: `${canonical.origin}${canonical.pathname.replace(/\/$/, "")}.md`,
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add MIME type to the markdown alternate link.

For alternate-representation discovery, include type: "text/markdown" on the link entry.

Proposed fix
           {
             rel: "alternate",
+            type: "text/markdown",
             href: `${canonical.origin}${canonical.pathname.replace(/\/$/, "")}.md`,
           },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{
rel: "alternate",
href: `${canonical.origin}${canonical.pathname.replace(/\/$/, "")}.md`,
},
{
rel: "alternate",
type: "text/markdown",
href: `${canonical.origin}${canonical.pathname.replace(/\/$/, "")}.md`,
},
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Head.astro` around lines 80 - 83, The alternate link object in
Head.astro (the entry with rel: "alternate" and href:
`${canonical.origin}${canonical.pathname.replace(/\/$/, "")}.md`) needs a MIME
type for markdown; add a property `type: "text/markdown"` to that link object so
the alternate-representation discovery explicitly declares the content type.

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