Skip to content

fix: hotfix for complex versions urls #1874

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.

Sign up for GitHub

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

Merged
danielroe merged 1 commit into from
Mar 3, 2026
Merged

fix: hotfix for complex versions urls #1874

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/utils/router.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ export function packageRoute(packageName: string, version?: string | null): Rout
params: {
org,
name,
version,
// remove spaces to be correctly resolved by router
version: version.replace(/\s+/g, ''),
Comment on lines +12 to +13
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

node-semver whitespace handling hyphen ranges OR unions npm semver

💡 Result:

node-semver (npm’s semver) whitespace rules: hyphen ranges + OR unions

1) Whitespace is AND (intersection), not OR

  • Comparators separated by spaces form a single comparator set that must all be true (AND). Example: >=1.2.7 <1.3.0. [1]

2) || is OR (union), and spaces around it don’t matter

  • A range is one or more comparator sets joined by || (OR). Example: 1.2.7 || >=1.2.9 <2.0.0. [1]
  • The grammar explicitly allows optional spaces around ||: ( ' ' ) * '||' ( ' ' ) *. [2]

3) Hyphen ranges require spaces around the hyphen

  • A hyphen range is defined as partial ' - ' partial (note the spaces). [2]
  • Example: 1.2.3 - 2.3.4 expands to >=1.2.3 <=2.3.4. [3]
  • If you remove the spaces (e.g. 1.2-1.4.5), many semver parsers interpret it as a prerelease (a single version) rather than a hyphen range. [4]

Practical takeaway

  • Use spaces for hyphen ranges: a.b.c - x.y.z
  • Use || (not spaces) for unions: range1 || range2
  • Use spaces between comparators for intersection: >=a <b

Sources: npm/node-semver README (ranges + grammar) [1][2][3], Masterminds semver whitespace note (illustrates the hyphen-spacing pitfall) [4].

🏁 Script executed:

# Find and examine the full context of the changes in app/utils/router.ts
fd -e ts -e tsx "router" | head -20

Repository: npmx-dev/npmx.dev

Length of output: 102

🏁 Script executed:

# Look for the specific file mentioned in the review
find . -name "router.ts" -o -name "router.tsx" 2>/dev/null | head -10

Repository: npmx-dev/npmx.dev

Length of output: 82

🏁 Script executed:

cat -n app/utils/router.ts

Repository: npmx-dev/npmx.dev

Length of output: 1295

Avoid stripping all whitespace from semver ranges.

At line 13, removing every whitespace character breaks valid semver semantics. Hyphen ranges like 1.2.3 - 2.3.4 require spaces around the hyphen; without them, parsers interpret the range as a prerelease version instead. Similarly, comparator sets like >=1.0.0 <2.0.0 depend on whitespace for correct intersection semantics.

🔧 Proposed fix 
   if (version) {
+    const normalisedVersion = version
+      .trim()
+      // normalise OR unions without mutating other valid range syntaxes
+      .replace(/\s*\|\|\s*/g, '||')
+
     return {
       name: 'package-version',
       params: {
         org,
         name,
-        // remove spaces to be correctly resolved by router
-        version: version.replace(/\s+/g, ''),
+        version: normalisedVersion,
       },
     }
   }

},
}
}
Expand Down
Loading