Skip to content

fix: markdown html links#1389

Merged
danielroe merged 5 commits intonpmx-dev:mainfrom
essenmitsosse:fix-markdown-links
Feb 11, 2026
Merged

fix: markdown html links#1389
danielroe merged 5 commits intonpmx-dev:mainfrom
essenmitsosse:fix-markdown-links

Conversation

@essenmitsosse
Copy link
Contributor

@essenmitsosse essenmitsosse commented Feb 11, 2026

As @alexdln noted: HTML links in markdown to npmjs.com where not converted to local links. In fact all most all processing for links only worked on markdown links. This moves the link transformation and analysis (for TOC + playground) to sanitize-html which parses the HTML after the Markdown has been converted to HTML, to have a single implementation for all links.

Addtional things:

  • Add TypeScript playground as playground
  • Fixes linked images having an external link icon

@vercel
Copy link

vercel bot commented Feb 11, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Feb 11, 2026 5:23pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 11, 2026 5:23pm
npmx-lunaria Ignored Ignored Feb 11, 2026 5:23pm

Request Review

@codecov
Copy link

codecov bot commented Feb 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@danielroe danielroe enabled auto-merge February 11, 2026 17:14
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 11, 2026

📝 Walkthrough

Walkthrough

The Readme.vue selector for external links now excludes anchors that contain an img descendant (:not(:has(img))), so icon-after styling applies only to non-image links. server/utils/readme.ts adds an optional path?: string to PlaygroundProvider, registers a TypeScript Playground provider with path: '/play', tightens provider matching to require path prefixes when present, introduces replaceHtmlLink to rewrite npmjs.com hrefs, attaches intermediate title metadata during token rendering, resolves/normalises hrefs during sanitise/transform, and collects playground link metadata for recognised providers.

Possibly related PRs

Suggested reviewers

  • alexdln
  • danielroe
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly relates to the changeset, detailing link processing changes, npmjs.com conversion, TypeScript playground addition, and fixing linked images with external link icons.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
server/utils/readme.ts (1)

414-422: Clean up redundant template literal and handle potential undefined value.

Two issues:

  1. Line 414: Double comment // // appears to be a leftover from incomplete cleanup.
  2. Line 420: The outer template literal interpolation is redundant, and plainText || title could evaluate to undefined if both are falsy, resulting in data-title-intermediate="undefined".
♻️ Suggested fix
-  // // Resolve link URLs, add security attributes, and collect playground links
+  // Add intermediate title data for playground link collection during sanitisation
   renderer.link = function ({ href, title, tokens }: Tokens.Link) {
     const text = this.parser.parseInline(tokens)
     const titleAttr = title ? ` title="${title}"` : ''
     const plainText = text.replace(/<[^>]*>/g, '').trim()
 
-    const intermediateTitleAttr = `${` data-title-intermediate="${plainText || title}"`}`
+    const intermediateTitleAttr = ` data-title-intermediate="${plainText || title || ''}"`
 
     return `<a href="${href}"${titleAttr}${intermediateTitleAttr}>${text}</a>`
   }

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

Copy link
Contributor

@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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
server/utils/readme.ts (1)

504-534: ⚠️ Potential issue | 🔴 Critical

Missing href assignment after resolution.

resolvedHref is computed and used for playground matching and external link detection, but it's never assigned back to attribs.href. The output will retain the original unresolved href, which means:

  • Anchor links won't get the user-content- prefix
  • Relative URLs won't be resolved to CDN/raw URLs
  • npmjs.com redirects won't be applied
🐛 Suggested fix
       const resolvedHref = resolveUrl(attribs.href, packageName, repoInfo)
+      attribs.href = resolvedHref

       const provider = matchPlaygroundProvider(resolvedHref)
🧹 Nitpick comments (1)
server/utils/readme.ts (1)

78-84: Minor: Capitalisation inconsistency in provider name.

"Typescript" should be "TypeScript" (capital S) to match the official branding.

✏️ Suggested fix
   {
     id: 'typescript-playground',
-    name: 'Typescript Playground',
+    name: 'TypeScript Playground',
     domains: ['typescriptlang.org'],
     path: '/play',
     icon: 'typescript',
   },

auto-merge was automatically disabled February 11, 2026 17:16

Head branch was pushed to by a user without write access

Copy link
Contributor

@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

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@danielroe danielroe added this pull request to the merge queue Feb 11, 2026
Merged via the queue into npmx-dev:main with commit ad306be Feb 11, 2026
18 checks passed
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