fix: 404 Cannot find any route matching, when adding a child route. #103
Conversation
src/router.ts
Outdated
| ? 0 | ||
| : a.maxDepth === remaining | ||
| ? -1 | ||
| : b.maxDepth === remaining |
There was a problem hiding this comment.
Those with the exact remaining have priority anyway.
|
Would you please add regression test too? (that fails without this fix) |
Sure, I'll do it tomorrow morning! is that ok? |
| testRouter( | ||
| [ | ||
| "/", | ||
| "/:packageAndRefOrSha", |
There was a problem hiding this comment.
This one triggers the if (node && node.placeholderChildren.length > 1) { condition.
|
@pi0 Here you go! Let me know what you think, I brought the exact test cases we have in https://github.com/stackblitz-labs/pkg.pr.new 😅 |
|
(keeping draft open for test reference) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v1 #103 +/- ##
=====================================
Coverage ? 70.14%
=====================================
Files ? 8
Lines ? 633
Branches ? 95
=====================================
Hits ? 444
Misses ? 185
Partials ? 4 ☔ View full report in Codecov by Sentry. |
Resolves unjs/nitro#2419
The maxDepth was more than
remainingin some cases. Whenserver/routes/[owner]/[repo]/[packageAndRefOrSha].get.tsandserver/routes/[owner]/[repo]/[npmOrg]/[packageAndRefOrSha].get.tsare present.The latter would affect the maxDepth, so in such cases, there might be children with lower maxDepth that are candidate. But
===did not work for those cases, so let's consider those with more maxDepth anyway.