Skip to content

Comments

feat: use router for import links in package code#1307

Merged
danielroe merged 2 commits intonpmx-dev:mainfrom
bluwy:package-code-import-links
Feb 10, 2026
Merged

feat: use router for import links in package code#1307
danielroe merged 2 commits intonpmx-dev:mainfrom
bluwy:package-code-import-links

Conversation

@bluwy
Copy link
Contributor

@bluwy bluwy commented Feb 10, 2026

Currently the import links in the package code page are raw anchor tags which aren't handled by Nuxt, which means they are handled by the browser and does a full page load when clicked.

I added some client-side code to attach click listeners and call navigateTo so that it uses our router for snappier navigation. I'm not sure if this is the best way but it might be the least complicated.

@vercel
Copy link

vercel bot commented Feb 10, 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 10, 2026 1:51am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 10, 2026 1:51am
npmx-lunaria Ignored Ignored Feb 10, 2026 1:51am

Request Review

@bluwy bluwy changed the title feat(package-code): use router for import links feat: use router for import links in package code Feb 10, 2026
@codecov
Copy link

codecov bot commented Feb 10, 2026

Codecov Report

❌ Patch coverage is 46.15385% with 7 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
app/components/Code/Viewer.vue 46.15% 6 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 10, 2026

📝 Walkthrough

Walkthrough

The change modifies the Code Viewer component to implement client-side navigation for import links. A new handleImportLinkNavigate function is introduced that attaches click listeners to anchor elements with the import-link class. When clicked, these links prevent their default browser behaviour and instead utilise Nuxt's navigateTo for routing. A watcher is added to the html prop to ensure the navigation handler runs again whenever the HTML content updates. No public APIs or component signatures are altered.

Suggested reviewers

  • danielroe
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description clearly explains the problem (raw anchor tags causing full page loads) and the solution (client-side navigation via navigateTo), directly matching the changeset.

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

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

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: 1

Comment on lines +62 to +74
function handleImportLinkNavigate() {
if (!codeRef.value) return

const anchors = codeRef.value.querySelectorAll('a.import-link')
anchors.forEach(anchor => {
// NOTE: We do not need to remove previous listeners because we re-create the entire HTML content on each html update
anchor.addEventListener('click', event => {
const href = anchor.getAttribute('href')
if (href) {
event.preventDefault()
navigateTo(href)
}
})
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n app/components/Code/Viewer.vue | sed -n '55,85p'

Repository: npmx-dev/npmx.dev

Length of output: 1006


🏁 Script executed:

head -n 61 app/components/Code/Viewer.vue | tail -n 40

Repository: npmx-dev/npmx.dev

Length of output: 1141


🏁 Script executed:

# Check if there are any other uses of addEventListener on import-link anchors
rg "import-link" app/components/Code/Viewer.vue

Repository: npmx-dev/npmx.dev

Length of output: 204


🏁 Script executed:

head -n 30 app/components/Code/Viewer.vue

Repository: npmx-dev/npmx.dev

Length of output: 836


🏁 Script executed:

# Verify the full component structure to check Vue Composition API usage
wc -l app/components/Code/Viewer.vue && echo "---" && tail -n 30 app/components/Code/Viewer.vue

Repository: npmx-dev/npmx.dev

Length of output: 911


🏁 Script executed:

head -n 100 app/components/Code/Viewer.vue | grep -E "^import|^const.*useTemplateRef|^const.*navigateTo"

Repository: npmx-dev/npmx.dev

Length of output: 102


🏁 Script executed:

sed -n '1,15p' app/components/Code/Viewer.vue

Repository: npmx-dev/npmx.dev

Length of output: 399


Preserve modified and auxiliary link behaviour before intercepting clicks.

Currently, all anchor clicks are prevented and routed via navigateTo, which blocks ctrl/cmd‑click, middle‑click, target="_blank", and download behaviours. Add guards so normal browser behaviour is preserved for these cases.

🔧 Suggested fix
   anchors.forEach(anchor => {
     // NOTE: We do not need to remove previous listeners because we re-create the entire HTML content on each html update
     anchor.addEventListener('click', event => {
+      if (
+        event.defaultPrevented ||
+        event.button !== 0 ||
+        event.metaKey ||
+        event.ctrlKey ||
+        event.shiftKey ||
+        event.altKey ||
+        (anchor.getAttribute('target') && anchor.getAttribute('target') !== '_self') ||
+        anchor.hasAttribute('download')
+      ) {
+        return
+      }
       const href = anchor.getAttribute('href')
       if (href) {
         event.preventDefault()
         navigateTo(href)
       }
     })
   })

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is sort of true I think, should we handle this? @danielroe

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made a fix locally. I'm not sure how merge queues work, so I'll wait for this to merge then send a follow-up PR.

@danielroe danielroe enabled auto-merge February 10, 2026 01:49
@danielroe danielroe added this pull request to the merge queue Feb 10, 2026
Merged via the queue into npmx-dev:main with commit d61b472 Feb 10, 2026
17 checks passed
@bluwy bluwy deleted the package-code-import-links branch February 10, 2026 02:29
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