Skip to content

Comments

feat: rewrite npmjs.org links#1573

Open
shamilkotta wants to merge 1 commit intonpmx-dev:mainfrom
shamilkotta:feat/npmjs-rewrite
Open

feat: rewrite npmjs.org links#1573
shamilkotta wants to merge 1 commit intonpmx-dev:mainfrom
shamilkotta:feat/npmjs-rewrite

Conversation

@shamilkotta
Copy link
Contributor

🔗 Linked issue

fixes: #1521

📚 Description

Rewrite to npmjs.org url to npmx relative path. currently we only have npmjs.com links handled

(Before and After)

Screen.Recording.mov

@vercel
Copy link

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

Request Review

@codecov
Copy link

codecov bot commented Feb 22, 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!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 22, 2026

📝 Walkthrough

Walkthrough

The changes extend npm.js URL redirect functionality by introducing a Set of multiple npmjs-related hostnames and updating the redirect logic to perform membership checks against this Set instead of direct host comparison. Additional unit tests verify that both npmjs.org domain variants (with and without www subdomain) are correctly redirected to local package paths, following the pattern href="/package/<pkg>".

Possibly related PRs

Suggested reviewers

  • alexdln
  • danielroe
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The PR description clearly describes the changes: rewriting npmjs.org URLs to npmx relative paths, fixing issue #1521, and extending existing npmjs.com link handling.
Linked Issues check ✅ Passed The PR successfully implements the core requirement from issue #1521 by adding support for npmjs.org domains in addition to existing npmjs.com handling, enabling URL rewrites to npmx.dev package pages.
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue: new npmJsHosts set, updated URL redirection logic, and corresponding test coverage for npmjs.org domains.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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.

🧹 Nitpick comments (1)
test/unit/server/utils/readme.spec.ts (1)

345-357: Consider adding a reserved-path exclusion test for npmjs.org.

The existing suite already verifies that https://www.npmjs.com/products is not redirected (line 338–343). There is no symmetrical assertion for the new .org TLD. Since isNpmJsUrlThatCanBeRedirected now admits both domains through the same path, a mirrored test would guard against any future regression where the exclusion is accidentally scoped only to .com.

✅ Suggested additional test
     it('redirects npmjs.org urls to local (no www and http)', async () => {
       const markdown = `[Some npmjs.org link](http://npmjs.org/package/test-pkg)`
       const result = await renderReadmeHtml(markdown, 'test-pkg')

       expect(result.html).toContain('href="/package/test-pkg"')
     })
+
+    it('does not redirect npmjs.org to local if they are in the list of exceptions', async () => {
+      const markdown = `[Some npmjs.org link](https://www.npmjs.org/products)`
+      const result = await renderReadmeHtml(markdown, 'test-pkg')
+
+      expect(result.html).toContain('href="https://www.npmjs.org/products"')
+    })
   })

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.

Revrite npmjs.com URLs in package readmes

1 participant