feat: replace links in markdown instead of via javascript (#1339)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Alex Savelyev <91429106+alexdln@users.noreply.github.com>
Co-authored-by: Daniel Roe <daniel@roe.dev>

Author: 4 people

app/components/Readme.vue

@@ -3,7 +3,6 @@ defineProps<{
   html: string
 }>()
 
-const router = useRouter()
 const { copy } = useClipboard()
 
 // Combined click handler for:
@@ -13,6 +12,10 @@ function handleClick(event: MouseEvent) {
   const target = event.target as HTMLElement | undefined
   if (!target) return
 
+  if (event.metaKey || event.ctrlKey || event.shiftKey || event.altKey || event.button) {
+    return
+  }
+
   // Handle copy button clicks
   const copyTarget = target.closest('[data-copy]')
   if (copyTarget) {
@@ -48,20 +51,11 @@ function handleClick(event: MouseEvent) {
   if (!href) return
 
   // Handle relative anchor links
-  if (href.startsWith('#')) {
+  if (href.startsWith('#') || href.startsWith('/')) {
     event.preventDefault()
-    router.push(href)
+    navigateTo(href)
     return
   }
-
-  const match = href.match(/^(?:https?:\/\/)?(?:www\.)?npmjs\.(?:com|org)(\/.+)$/)
-  if (!match || !match[1]) return
-
-  const route = router.resolve(match[1])
-  if (route) {
-    event.preventDefault()
-    router.push(route)
-  }
 }
 </script>
 
@@ -141,15 +135,19 @@ function handleClick(event: MouseEvent) {
 }
 
 .readme :deep(a) {
-  color: var(--fg);
-  text-decoration: underline;
-  text-underline-offset: 4px;
-  text-decoration-color: var(--fg-subtle);
-  transition: text-decoration-color 0.2s ease;
+  @apply underline-offset-[0.2rem] underline decoration-1 decoration-fg/30 font-mono text-fg transition-colors duration-200;
 }
-
 .readme :deep(a:hover) {
-  text-decoration-color: var(--accent);
+  @apply decoration-accent text-accent;
+}
+.readme :deep(a:focus-visible) {
+  @apply decoration-accent text-accent;
+}
+
+.readme :deep(a[target='_blank']::after) {
+  /* I don't know what kind of sorcery this is, but it ensures this icon can't wrap to a new line on its own. */
+  content: '__';
+  @apply inline i-carbon:launch rtl-flip ms-1 opacity-50;
 }
 
 .readme :deep(code) {

server/utils/readme.ts

@@ -183,6 +183,33 @@ function slugify(text: string): string {
     .replace(/^-|-$/g, '') // Trim leading/trailing hyphens
 }
 
+/** These path on npmjs.com don't belong to packages or search, so we shouldn't try to replace them with npmx.dev urls */
+const reservedPathsNpmJs = [
+  'products',
+  'login',
+  'signup',
+  'advisories',
+  'blog',
+  'about',
+  'press',
+  'policies',
+]
+
+const isNpmJsUrlThatCanBeRedirected = (url: URL) => {
+  if (url.host !== 'www.npmjs.com' && url.host !== 'npmjs.com') {
+    return false
+  }
+
+  if (
+    url.pathname === '/' ||
+    reservedPathsNpmJs.some(path => url.pathname.startsWith(`/${path}`))
+  ) {
+    return false
+  }
+
+  return true
+}
+
 /**
  * Resolve a relative URL to an absolute URL.
  * If repository info is available, resolve to provider's raw file URLs.
@@ -199,6 +226,10 @@ function resolveUrl(url: string, packageName: string, repoInfo?: RepositoryInfo)
     try {
       const parsed = new URL(url, 'https://example.com')
       if (parsed.protocol === 'http:' || parsed.protocol === 'https:') {
+        // Redirect npmjs urls to ourself
+        if (isNpmJsUrlThatCanBeRedirected(parsed)) {
+          return parsed.pathname + parsed.search + parsed.hash
+        }
         return url
       }
     } catch {

test/unit/server/utils/readme.spec.ts

@@ -306,6 +306,29 @@ describe('Markdown File URL Resolution', () => {
       )
     })
   })
+
+  describe('npm.js urls', () => {
+    it('redirects npmjs.com urls to local', async () => {
+      const markdown = `[Some npmjs.com link](https://www.npmjs.com/package/test-pkg)`
+      const result = await renderReadmeHtml(markdown, 'test-pkg')
+
+      expect(result.html).toContain('href="/package/test-pkg"')
+    })
+
+    it('redirects npmjs.com urls to local (no www and http)', async () => {
+      const markdown = `[Some npmjs.com link](http://npmjs.com/package/test-pkg)`
+      const result = await renderReadmeHtml(markdown, 'test-pkg')
+
+      expect(result.html).toContain('href="/package/test-pkg"')
+    })
+
+    it('does not redirect npmjs.com to local if they are in the list of exceptions', async () => {
+      const markdown = `[Root Contributing](https://www.npmjs.com/products)`
+      const result = await renderReadmeHtml(markdown, 'test-pkg')
+
+      expect(result.html).toContain('href="https://www.npmjs.com/products"')
+    })
+  })
 })
 
 describe('Markdown Content Extraction', () => {