fix(intent): decode legacy __ scoped-package slugs (500 on indexed registry URLs)#1051
Open
dearlordylord wants to merge 1 commit into
Open
fix(intent): decode legacy __ scoped-package slugs (500 on indexed registry URLs)#1051dearlordylord wants to merge 1 commit into
__ scoped-package slugs (500 on indexed registry URLs)#1051dearlordylord wants to merge 1 commit into
Conversation
Legacy registry links encoded the scope separator as __ (@scope/name -> @scope__name). Those URLs are still indexed; landing on one ran the package lookup with the literal @scope__name, failing npm package-name validation and returning HTTP 500. Restore the __ -> / scope-separator back-compat in decodePackageNameSlug, mirroring src/routes/stats/npm/-utils.ts. Unscoped names and names that legitimately contain __ are left intact. Refs TanStack#1050
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesRoute encoding compatibility
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Legacy registry links encoded the scope separator as
__(@scope/name→@scope__name) and those URLs are still indexed by search engines. Commit 0f55472 switched the slug codec toencodeURIComponent/decodeURIComponentand dropped the old__→/decode. Landing on a legacy__URL now runs the package lookup with the literal@scope__name, which fails the npm package-name regex → the query errors → HTTP 500.Reproduces for every scoped package via its legacy URL, e.g.:
Fix
Restore the
__→/scope-separator back-compat indecodePackageNameSlug, mirroring the still-present logic insrc/routes/stats/npm/-utils.ts(decodePackageName). The guard is narrow —startsWith('@') && includes('__') && !includes('/')— so:@firfi__quint-connect→@firfi/quint-connect@depup__trpc__server→@depup/trpc__server(only the scope separator is restored)tanstack__intent→ unchanged%2F/ double-encoded slugs → unchangedTests
Added legacy-slug cases to
tests/route-encoding.test.ts.Local gate (all green):
pnpm test(tsc+oxlint --type-aware+tsx --test) andpnpm build.Not included (open to feedback)
A 301 redirect from the legacy
__slug to the canonicalencodeURIComponentURL would additionally consolidate SEO. Left out here because the router's double-encoding makes a redirect loop-prone; this decoder fix alone stops the 500s. Happy to add the redirect if preferred.Closes #1050
Summary by CodeRabbit