Skip to content

fix: replace hardcoded npmx.dev & docs.npmx.dev with env-specific URLs#1402

Draft
serhalp wants to merge 1 commit intomainfrom
fix/per-env-self-urls
Draft

fix: replace hardcoded npmx.dev & docs.npmx.dev with env-specific URLs#1402
serhalp wants to merge 1 commit intomainfrom
fix/per-env-self-urls

Conversation

@serhalp
Copy link
Contributor

@serhalp serhalp commented Feb 12, 2026

Extract a composable that returns URLs for this app itself (and docs, which lives in this codebase), so that URLs are always internally consistent within an environment, e.g. in dev links go to dev.

Extract a composable that returns URLs for this app itself (and docs, which lives in this codebase),
so that URLs are always internally consistent within an environment, e.g. in dev links go to dev.
@vercel
Copy link

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

Request Review

@codecov
Copy link

codecov bot commented Feb 12, 2026

Codecov Report

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

Files with missing lines Patch % Lines
app/app.vue 0.00% 2 Missing ⚠️
app/components/Package/SkillsModal.vue 0.00% 1 Missing and 1 partial ⚠️
app/composables/useJsonLd.ts 0.00% 1 Missing ⚠️
app/pages/package/[[org]]/[name].vue 0.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@serhalp serhalp marked this pull request as ready for review February 12, 2026 00:58
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 12, 2026

📝 Walkthrough

Walkthrough

The pull request centralises URL configuration by introducing a new useAppUrls() composable that provides dynamic site and documentation URLs. Hard-coded URLs across multiple components and pages are replaced with calls to this new composable. The createWebSiteSchema() function signature is updated to accept a siteUrl parameter. Environment configuration is extended to expose siteUrl through the application configuration via getEnv(). Constants are renamed from NPMX_SITE to NPMX_SITE_PROD and a new NPMX_DOCS_SITE_PROD constant is added to shared utilities.

Possibly related PRs

Suggested reviewers

  • danielroe
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description accurately describes the changeset: extracting a composable for environment-specific URLs to replace hardcoded site references.

✏️ 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
  • Commit unit tests in branch fix/per-env-self-urls

No actionable comments were generated in the recent review. 🎉

Tip

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


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

: undefined

export const getSiteUrl = (isDevelopment: boolean) => {
if (isDevelopment) return 'http://localhost:3000'
Copy link
Contributor

@knowler knowler Feb 12, 2026

Choose a reason for hiding this comment

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

wondering if we should or should not hardcode this? It is possible to pass flags with the dev command to change the port/host. Could we get that from somewhere else?

Copy link
Member

Choose a reason for hiding this comment

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

unfortunately only 127.0.0.1 works for the atproto oauth callback

but yes, you can get the actual host and port in module space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as I have a feature PR stacked on this one and PRs get stale fast here 😅 and this PR leaves things no worse than before, how would you feel about following up on this later?

return {
siteUrl,
// TODO(serhalp): Handle preview environment. The docs site is a separate deployment, so we'd
// need to infer its preview URL from the site preview URL somehow...?
Copy link
Contributor

Choose a reason for hiding this comment

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

https://vercel.com/docs/deployments/generated-urls#generated-from-git

the URL's are deterministic, so can we just construct them during the build?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as I have a feature PR stacked on this one and PRs get stale fast here 😅 and this PR leaves things no worse than before, how would you feel about following up on this later?

@whitep4nth3r whitep4nth3r marked this pull request as draft February 12, 2026 15:47
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.

4 participants