search

unjs / rou3

🌳 Lightweight and fast rou(ter) for JavaScript
MIT License
442 stars 16 forks source link

feat!: ignore queryparams in matching #61

Closed kingtimm closed 3 months ago

kingtimm commented 1 year ago

🔗 Linked issue

fixes #54 - inclusion of queryparameters on a path make it fail to match.

❓ Type of change

📚 Description

Removed query parameters from the path before it is matched. The router did not seem to support them anyway.

My use-case was in nuxt-vitest's registerEndpoint but found others running into it.

Resolves #54

📝 Checklist

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.66%. Comparing base (db3a623) to head (35d94e5). Report is 10 commits behind head on v2.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## v2 #61 +/- ## ========================================== + Coverage 95.63% 95.66% +0.02% ========================================== Files 4 4 Lines 435 438 +3 Branches 84 84 ========================================== + Hits 416 419 +3 Misses 19 19 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

pi0 commented 10 months ago

Thanks for PR dear @kingtimm and sorry it is delayed. Main reason is that, the consumer libs (mainly unjs/h3) are already doing this normalization. It is not a breaking-change per-se but doing this means performance overhead of iterating string twice. Because of this i plan this in next major release to be included so we can adjust h3 router behavior while upgrading.

mukundshah commented 4 months ago

I suggest this should be optional. I some case, we may need to match the route along with the query params;

Maybe having something like {query: true} and false being the default value, we could have this without breaking anything.

pi0 commented 3 months ago

Hi dear @kingtimm. I had been thinking about this for a while and while in some direct usecases it makes sense, i think it would be the best to let it up to the users to remove query params and pass pathname themselves.

In cases of using with web APIs, if users already parsed URL with new URL, they already have access to url.pathname. This improves perf, reusability (if we drop internally in rou3, users need to parse again) and also stability since URL parsing has many edge cases.