feat: add npmjs.dev to playground links (#2314)#2495
feat: add npmjs.dev to playground links (#2314)#2495romansp wants to merge 1 commit intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThe change modifies the Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/pages/package/[[org]]/[name].vue (1)
75-89: Avoid duplicate playground entries before pushing links.On Line 75 and Line 84, links are appended without dedupe checks. If the README already contains the same URL, this can create duplicate items and duplicate command IDs (URL-based) in
app/components/Package/Playgrounds.vue(Lines 121-132).Proposed patch
- if (packageData.storybook?.url) { + if (packageData.storybook?.url && !links.some(link => link.url === packageData.storybook.url)) { links.push({ url: packageData.storybook.url, provider: 'storybook', providerName: 'Storybook', label: 'Storybook', }) } - links.push({ - url: `https://npmjs.dev/${packageData.name}`, - provider: 'npmjs.dev', - providerName: 'npmjs.dev', - label: 'npmjs.dev', - }) + const npmjsDevUrl = `https://npmjs.dev/${packageData.name}` + if (!links.some(link => link.url === npmjsDevUrl)) { + links.push({ + url: npmjsDevUrl, + provider: 'npmjs.dev', + providerName: 'npmjs.dev', + label: 'npmjs.dev', + }) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/package/`[[org]]/[name].vue around lines 75 - 89, The code pushes new playground links into the links array (e.g., the Storybook block and the npmjs.dev push) without checking for duplicates, which can create duplicate items and duplicate URL-based command IDs in Playgrounds.vue; update the logic around the links.push calls to first check whether a link with the same url (or same provider+url) already exists in links (e.g., via links.some(l => l.url === packageData.storybook.url) or similar) and only push the new entry if not present, ensuring both the Storybook and npmjs.dev additions are guarded to prevent duplicate entries/IDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/pages/package/`[[org]]/[name].vue:
- Around line 75-89: The code pushes new playground links into the links array
(e.g., the Storybook block and the npmjs.dev push) without checking for
duplicates, which can create duplicate items and duplicate URL-based command IDs
in Playgrounds.vue; update the logic around the links.push calls to first check
whether a link with the same url (or same provider+url) already exists in links
(e.g., via links.some(l => l.url === packageData.storybook.url) or similar) and
only push the new entry if not present, ensuring both the Storybook and
npmjs.dev additions are guarded to prevent duplicate entries/IDs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8be95c8d-e09d-4d4e-a811-9333354e38aa
📒 Files selected for processing (1)
app/pages/package/[[org]]/[name].vue
ghostdevv
left a comment
ghostdevv
left a comment
There was a problem hiding this comment.
It should be added to the command palette too - also since the name npmjs.dev doesn't really describe what it is we may need to add a different description
|
Thank you for your review @ghostdevv. Double-checked and all playground links are added automatically to the command palette here https://github.com/romansp/npmx.dev/blob/e74c83beac7443003a07612dbf1d5513a57a2352/app/components/Package/Playgrounds.vue#L121-L132. No additional changes are required. Agreed that description should be updated. Here's other variants I considered, let me know which one fits better in your opinion.
Here's how it looks with "Try on npmjs.dev". |
2 participants
🔗 Linked issue
Resolves #2314.
🧭 Context
Simple solution to provide "Try on RunKit" alternative by linking to
https://npmjs.dev/{packageName]and keep "how to run package in browser" out of npmx.dev scope.📚 Description
Adds "npmjs.dev" to collapsible "Try it out" section in package sidebar. This PR makes a change to always add link to ad-hoc playground on
npmjs.dev. Link is added last to the list and will always appear after official playgrounds, e.g. CodeSandbox or Storybook.I originally considered to add this link to
ExternalLinks.vuebut "Playgrounds" seems to be a perfect fit for it. Downside of this solution is that "Try it out" section is now always present, which pushes Versions section down.Open to hear feedback and iterate on this solution.