-
Notifications
You must be signed in to change notification settings - Fork 20
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.
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
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
Co-Authored-By: Mohammad Bagher Abiyat <zorofight94@gmail.com>
Hey. I'm back to radix3! Added your nice tests via 1747ff7 to main (v2) branch as first step. Will try to investigate more. |
Thank you so much! ❤️ |
Should be fixed after. #107 |
Resolves nitrojs/nitro#2419
The maxDepth was more than
remaining
in some cases. Whenserver/routes/[owner]/[repo]/[packageAndRefOrSha].get.ts
andserver/routes/[owner]/[repo]/[npmOrg]/[packageAndRefOrSha].get.ts
are 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.