fix(withoutTrailingSlash): consider qurry param #219
Conversation
src/utils.ts
Outdated
| @@ -84,15 +84,15 @@ export function hasTrailingSlash( | |||
| respectQueryAndFragment?: boolean | |||
| ): boolean { | |||
| if (!respectQueryAndFragment) { | |||
| return input.endsWith("/"); | |||
| return !input.includes("?") && input.endsWith("/"); | |||
There was a problem hiding this comment.
(notice that by default, we do not handle query. we probably don't want to change logic in this line)
There was a problem hiding this comment.
i see that query is not handled by default, but what to do with these cases?
because, currently, all these calls return true:
hasTrailingSlash('/a?b=/', false)
hasTrailingSlash('/a/?b=/', false)
hasTrailingSlash('/a?b=/', true)
hasTrailingSlash('/a/?b=/', true)There was a problem hiding this comment.
Checking, without this change, only failing test is withoutTrailingSlash('foo/?k=/', false /* no respect */). (test in L88 still passes)
There was a problem hiding this comment.
I have revert it for now since change on hasProcol like this is risky to introduce behavior changes.
pi0
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #219 +/- ##
==========================================
+ Coverage 95.27% 98.69% +3.42%
==========================================
Files 7 7
Lines 867 1378 +511
Branches 187 230 +43
==========================================
+ Hits 826 1360 +534
+ Misses 41 18 -23 ☔ View full report in Codecov by Sentry. |
🔗 Linked issue
Fixes #214
❓ Type of change
📝 Checklist