-
-
Notifications
You must be signed in to change notification settings - Fork 240
fix: navigating to org page from search leads to 404 page #1191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Org packages are fetched from the npm registry api (`/-/org/{org}/package`) in two places:
1. Server route `/api/registry/org/[org]/packages.get.ts`:
- calls `$fetch('https://registry.npmjs.org/-/org/{org}/package')`
2. Client composable `useOrgPackages()`: calls `$npmRegistry('/-/org/{org}/package')`
The composable bypasses the server route, duplicating the registry call, org name encoding, and
error handling, and bypassing the important server-side caching. But most importantly, calls to the
registry from the client fail with a CORS error, since the registry doesn't allow cross-origin
requests from browsers.
📝 WalkthroughWalkthroughThe pull request updates the organisation packages retrieval flow across both client and server layers. The client-side composable is modified to consume a new API endpoint that returns packages and count directly, replacing a previously wrapped data structure. The server-side API endpoint introduces specific error handling for 404 responses from the registry, propagating "Organisation not found" errors to clients, whilst non-404 errors are logged and result in an empty package list. This distinction allows consumers to differentiate between missing organisations and empty results. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
|
oh hmm wait a sec, this is effectively reverting a change from #641? 👀 |
|
ahh, probably an unintentional change it was an older PR I merged stuff into. |
Bug
See #1190.
Fix
Org packages are fetched from the npm registry api (
/-/org/{org}/package) in two places:/api/registry/org/[org]/packages.get.ts: calls$fetch('https://registry.npmjs.org/-/org/{org}/package')useOrgPackages(): calls$npmRegistry('/-/org/{org}/package')The composable bypasses the server route, duplicating the registry call, org name encoding, and error handling, and bypassing the important server-side caching. But most importantly, calls to the registry from the client fail with a CORS error, since the registry doesn't allow cross-origin requests from browsers.
Fixes #1190.
QA
Note: if you try this fix, please note that if you use an org that has many packages you'll likely run into a separate issue that prints a ton of 429s (with CORS errors) in your browser console. This is a separate latent issue. Let's fix that in a separate PR 😅.