From 03af326cf6de5d5531d171ab773ca87e406d9262 Mon Sep 17 00:00:00 2001 From: Mohammad Bagher Abiyat Date: Sat, 11 May 2024 01:37:41 +0330 Subject: [PATCH 1/8] init --- src/router.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/router.ts b/src/router.ts index d36d3bc..d354b3f 100644 --- a/src/router.ts +++ b/src/router.ts @@ -66,8 +66,19 @@ function lookup( if (node && node.placeholderChildren.length > 1) { // https://github.com/unjs/radix3/issues/95 const remaining = sections.length - i; + + const sortedPlaceholderChildren = [...node.placeholderChildren].sort( + (a, b) => + a.maxDepth === remaining && b.maxDepth === remaining + ? 0 + : a.maxDepth === remaining + ? -1 + : b.maxDepth === remaining + ? 1 + : b.maxDepth - a.maxDepth, + ); node = - node.placeholderChildren.find((c) => c.maxDepth === remaining) || + sortedPlaceholderChildren.find((c) => c.maxDepth >= remaining) || null; } else { node = node.placeholderChildren[0] || null; From 81d068d32c2773d0a52d6afbd05319e4d11b4750 Mon Sep 17 00:00:00 2001 From: Mohammad Bagher Abiyat Date: Sat, 11 May 2024 09:16:25 +0330 Subject: [PATCH 2/8] add tests --- src/router.ts | 4 ++-- tests/router.test.ts | 28 ++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/router.ts b/src/router.ts index d354b3f..a805b0f 100644 --- a/src/router.ts +++ b/src/router.ts @@ -71,11 +71,11 @@ function lookup( (a, b) => a.maxDepth === remaining && b.maxDepth === remaining ? 0 - : a.maxDepth === remaining + : (a.maxDepth === remaining ? -1 : b.maxDepth === remaining ? 1 - : b.maxDepth - a.maxDepth, + : b.maxDepth - a.maxDepth), ); node = sortedPlaceholderChildren.find((c) => c.maxDepth >= remaining) || diff --git a/tests/router.test.ts b/tests/router.test.ts index 670a7af..c7e2ca7 100644 --- a/tests/router.test.ts +++ b/tests/router.test.ts @@ -135,6 +135,34 @@ describe("Router lookup", function () { "route/with/trailing/slash": { path: "route/with/trailing/slash/" }, }); }); + + describe("routes with lower maxDepth should be considered too", function () { + testRouter( + [ + "route/:owner/:repo/:packageAndRefOrSha", + "route/:owner/:repo/:npmOrg/:packageAndRefOrSha", + ], + { + "route/tinylibs/tinybench/tiny@232": { + path: "route/:owner/:repo/:packageAndRefOrSha", + params: { + owner: "tinylibs", + repo: "tinybench", + packageAndRefOrSha: "tiny@232", + }, + }, + "route/tinylibs/tinybench/@tinylibs/tiny@232": { + path: "route/:owner/:repo/:npmOrg/:packageAndRefOrSha", + params: { + owner: "tinylibs", + repo: "tinybench", + npmOrg: "@tinylibs", + packageAndRefOrSha: "tiny@232", + }, + }, + }, + ); + }); }); describe("Router insert", function () { From db2db142635dc782ffda151731b51bf6d359a2af Mon Sep 17 00:00:00 2001 From: Mohammad Bagher Abiyat Date: Sat, 11 May 2024 09:21:35 +0330 Subject: [PATCH 3/8] update tests --- src/router.ts | 13 +------------ tests/router.test.ts | 3 +++ 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/router.ts b/src/router.ts index a805b0f..d36d3bc 100644 --- a/src/router.ts +++ b/src/router.ts @@ -66,19 +66,8 @@ function lookup( if (node && node.placeholderChildren.length > 1) { // https://github.com/unjs/radix3/issues/95 const remaining = sections.length - i; - - const sortedPlaceholderChildren = [...node.placeholderChildren].sort( - (a, b) => - a.maxDepth === remaining && b.maxDepth === remaining - ? 0 - : (a.maxDepth === remaining - ? -1 - : b.maxDepth === remaining - ? 1 - : b.maxDepth - a.maxDepth), - ); node = - sortedPlaceholderChildren.find((c) => c.maxDepth >= remaining) || + node.placeholderChildren.find((c) => c.maxDepth === remaining) || null; } else { node = node.placeholderChildren[0] || null; diff --git a/tests/router.test.ts b/tests/router.test.ts index c7e2ca7..8952046 100644 --- a/tests/router.test.ts +++ b/tests/router.test.ts @@ -139,6 +139,9 @@ describe("Router lookup", function () { describe("routes with lower maxDepth should be considered too", function () { testRouter( [ + "route/", + "route/:packageAndRefOrSha", + "route/:owner/:repo/", "route/:owner/:repo/:packageAndRefOrSha", "route/:owner/:repo/:npmOrg/:packageAndRefOrSha", ], From 82fbfc9dda8ba1c6fefcdd20aca303b7c17287fd Mon Sep 17 00:00:00 2001 From: Mohammad Bagher Abiyat Date: Sat, 11 May 2024 09:30:25 +0330 Subject: [PATCH 4/8] update tests --- src/router.ts | 23 ++++++++++++++++++++++- tests/router.test.ts | 18 +++++++++--------- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/router.ts b/src/router.ts index d36d3bc..48a1db4 100644 --- a/src/router.ts +++ b/src/router.ts @@ -66,8 +66,29 @@ function lookup( if (node && node.placeholderChildren.length > 1) { // https://github.com/unjs/radix3/issues/95 const remaining = sections.length - i; + + const sortedPlaceholderChildren = [...node.placeholderChildren].sort( + (a, b) => { + // If both items have maxDepth equal to remaining, no change in order + if (a.maxDepth === remaining && b.maxDepth === remaining) { + return 0; + } + // If only item a has maxDepth equal to remaining, it should come before b + else if (a.maxDepth === remaining) { + return -1; + } + // If only item b has maxDepth equal to remaining, it should come after a + else if (b.maxDepth === remaining) { + return 1; + } + // If maxDepth is not equal to remaining for both items, sort based on their maxDepth + else { + return b.maxDepth - a.maxDepth; + } + }, + ); node = - node.placeholderChildren.find((c) => c.maxDepth === remaining) || + sortedPlaceholderChildren.find((c) => c.maxDepth >= remaining) || null; } else { node = node.placeholderChildren[0] || null; diff --git a/tests/router.test.ts b/tests/router.test.ts index 8952046..770580c 100644 --- a/tests/router.test.ts +++ b/tests/router.test.ts @@ -139,23 +139,23 @@ describe("Router lookup", function () { describe("routes with lower maxDepth should be considered too", function () { testRouter( [ - "route/", - "route/:packageAndRefOrSha", - "route/:owner/:repo/", - "route/:owner/:repo/:packageAndRefOrSha", - "route/:owner/:repo/:npmOrg/:packageAndRefOrSha", + // "/", + // "/:packageAndRefOrSha", + "/:owner/:repo/", + "/:owner/:repo/:packageAndRefOrSha", + "/:owner/:repo/:npmOrg/:packageAndRefOrSha", ], { - "route/tinylibs/tinybench/tiny@232": { - path: "route/:owner/:repo/:packageAndRefOrSha", + "/tinylibs/tinybench/tiny@232": { + path: "/:owner/:repo/:packageAndRefOrSha", params: { owner: "tinylibs", repo: "tinybench", packageAndRefOrSha: "tiny@232", }, }, - "route/tinylibs/tinybench/@tinylibs/tiny@232": { - path: "route/:owner/:repo/:npmOrg/:packageAndRefOrSha", + "/tinylibs/tinybench/@tinylibs/tiny@232": { + path: "/:owner/:repo/:npmOrg/:packageAndRefOrSha", params: { owner: "tinylibs", repo: "tinybench", From 8e89939465b5067d469d6ffcf0f261fbe1650b80 Mon Sep 17 00:00:00 2001 From: Mohammad Bagher Abiyat Date: Sat, 11 May 2024 12:32:12 +0330 Subject: [PATCH 5/8] guess this is the final fix --- src/router.ts | 42 +++++++++++++++++++++++++++++++----------- tests/router.test.ts | 4 ++-- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/router.ts b/src/router.ts index 48a1db4..e031e7e 100644 --- a/src/router.ts +++ b/src/router.ts @@ -22,6 +22,7 @@ export function createRouter( if (options.routes) { for (const path in options.routes) { + console.log("insert", path); insert(ctx, normalizeTrailingSlash(path), options.routes[path]); } } @@ -45,6 +46,7 @@ function lookup( } const sections = path.split("/"); + console.log("sections", sections); const params: MatchedRoute["params"] = {}; let paramsFound = false; @@ -54,6 +56,7 @@ function lookup( for (let i = 0; i < sections.length; i++) { const section = sections[i]; + console.log(i, "section", section, node); if (node.wildcardChildNode !== null) { wildcardNode = node.wildcardChildNode; @@ -64,36 +67,37 @@ function lookup( const nextNode = node.children.get(section); if (nextNode === undefined) { if (node && node.placeholderChildren.length > 1) { + console.log("node && node.placeholderChildren.length > 1"); // https://github.com/unjs/radix3/issues/95 const remaining = sections.length - i; - + console.log("i", i, "remaining", remaining, "sections", sections); + console.log("node.placeholderChildren", node.placeholderChildren); + // prioritize items with the exact maxDepth as remaining const sortedPlaceholderChildren = [...node.placeholderChildren].sort( (a, b) => { - // If both items have maxDepth equal to remaining, no change in order if (a.maxDepth === remaining && b.maxDepth === remaining) { return 0; } - // If only item a has maxDepth equal to remaining, it should come before b else if (a.maxDepth === remaining) { return -1; } - // If only item b has maxDepth equal to remaining, it should come after a else if (b.maxDepth === remaining) { return 1; } - // If maxDepth is not equal to remaining for both items, sort based on their maxDepth else { return b.maxDepth - a.maxDepth; } }, ); + node = - sortedPlaceholderChildren.find((c) => c.maxDepth >= remaining) || - null; + sortedPlaceholderChildren.find((c) => c.maxDepth >= remaining) || null; } else { + console.log("here"); node = node.placeholderChildren[0] || null; } if (!node) { + console.log("unfound node"); break; } if (node.paramName) { @@ -159,14 +163,30 @@ function insert(ctx: RadixRouterContext, path: string, data: any) { childNode.paramName = section.slice(3 /* "**:" */) || "_"; isStaticRoute = false; } - - matchedNodes.push(childNode); node = childNode; } + matchedNodes.push(childNode); } - for (const [depth, node] of matchedNodes.entries()) { - node.maxDepth = Math.max(matchedNodes.length - depth, node.maxDepth || 0); + console.log(path, "matchedNodes length", matchedNodes.length); + for (const [depth, matchedNode] of matchedNodes.entries()) { + const prev = matchedNode.maxDepth; + + matchedNode.maxDepth = Math.max( + matchedNodes.length - depth, + matchedNode.maxDepth || 0, + ); + console.log("sections", sections, "depth", depth); + console.log( + "prev node.maxDepth", + prev, + "maybe", + matchedNodes.length - depth, + "node.maxDepth", + sections[depth] ?? " ", + matchedNode.maxDepth, + ); + // node.maxDepth = matchedNodes.length - depth; } // Store whatever data was provided into the node diff --git a/tests/router.test.ts b/tests/router.test.ts index 770580c..5e5d7a2 100644 --- a/tests/router.test.ts +++ b/tests/router.test.ts @@ -139,8 +139,8 @@ describe("Router lookup", function () { describe("routes with lower maxDepth should be considered too", function () { testRouter( [ - // "/", - // "/:packageAndRefOrSha", + "/", + "/:packageAndRefOrSha", "/:owner/:repo/", "/:owner/:repo/:packageAndRefOrSha", "/:owner/:repo/:npmOrg/:packageAndRefOrSha", From 72378536d4ba3d86edb8c979372789f1998853a9 Mon Sep 17 00:00:00 2001 From: Mohammad Bagher Abiyat Date: Sat, 11 May 2024 12:32:56 +0330 Subject: [PATCH 6/8] without fix --- src/router.ts | 53 ++++++--------------------------------------------- 1 file changed, 6 insertions(+), 47 deletions(-) diff --git a/src/router.ts b/src/router.ts index e031e7e..d36d3bc 100644 --- a/src/router.ts +++ b/src/router.ts @@ -22,7 +22,6 @@ export function createRouter( if (options.routes) { for (const path in options.routes) { - console.log("insert", path); insert(ctx, normalizeTrailingSlash(path), options.routes[path]); } } @@ -46,7 +45,6 @@ function lookup( } const sections = path.split("/"); - console.log("sections", sections); const params: MatchedRoute["params"] = {}; let paramsFound = false; @@ -56,7 +54,6 @@ function lookup( for (let i = 0; i < sections.length; i++) { const section = sections[i]; - console.log(i, "section", section, node); if (node.wildcardChildNode !== null) { wildcardNode = node.wildcardChildNode; @@ -67,37 +64,15 @@ function lookup( const nextNode = node.children.get(section); if (nextNode === undefined) { if (node && node.placeholderChildren.length > 1) { - console.log("node && node.placeholderChildren.length > 1"); // https://github.com/unjs/radix3/issues/95 const remaining = sections.length - i; - console.log("i", i, "remaining", remaining, "sections", sections); - console.log("node.placeholderChildren", node.placeholderChildren); - // prioritize items with the exact maxDepth as remaining - const sortedPlaceholderChildren = [...node.placeholderChildren].sort( - (a, b) => { - if (a.maxDepth === remaining && b.maxDepth === remaining) { - return 0; - } - else if (a.maxDepth === remaining) { - return -1; - } - else if (b.maxDepth === remaining) { - return 1; - } - else { - return b.maxDepth - a.maxDepth; - } - }, - ); - node = - sortedPlaceholderChildren.find((c) => c.maxDepth >= remaining) || null; + node.placeholderChildren.find((c) => c.maxDepth === remaining) || + null; } else { - console.log("here"); node = node.placeholderChildren[0] || null; } if (!node) { - console.log("unfound node"); break; } if (node.paramName) { @@ -163,30 +138,14 @@ function insert(ctx: RadixRouterContext, path: string, data: any) { childNode.paramName = section.slice(3 /* "**:" */) || "_"; isStaticRoute = false; } + + matchedNodes.push(childNode); node = childNode; } - matchedNodes.push(childNode); } - console.log(path, "matchedNodes length", matchedNodes.length); - for (const [depth, matchedNode] of matchedNodes.entries()) { - const prev = matchedNode.maxDepth; - - matchedNode.maxDepth = Math.max( - matchedNodes.length - depth, - matchedNode.maxDepth || 0, - ); - console.log("sections", sections, "depth", depth); - console.log( - "prev node.maxDepth", - prev, - "maybe", - matchedNodes.length - depth, - "node.maxDepth", - sections[depth] ?? " ", - matchedNode.maxDepth, - ); - // node.maxDepth = matchedNodes.length - depth; + for (const [depth, node] of matchedNodes.entries()) { + node.maxDepth = Math.max(matchedNodes.length - depth, node.maxDepth || 0); } // Store whatever data was provided into the node From 016de58c2e27abf36fedb8bbef48f2cf1ca51b94 Mon Sep 17 00:00:00 2001 From: Mohammad Bagher Abiyat Date: Sat, 11 May 2024 12:35:55 +0330 Subject: [PATCH 7/8] with fix --- src/router.ts | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/router.ts b/src/router.ts index d36d3bc..aff1e7b 100644 --- a/src/router.ts +++ b/src/router.ts @@ -66,9 +66,26 @@ function lookup( if (node && node.placeholderChildren.length > 1) { // https://github.com/unjs/radix3/issues/95 const remaining = sections.length - i; + // prioritize items with the exact maxDepth as remaining + const sortedPlaceholderChildren = [...node.placeholderChildren].sort( + (a, b) => { + if (a.maxDepth === remaining && b.maxDepth === remaining) { + return 0; + } + else if (a.maxDepth === remaining) { + return -1; + } + else if (b.maxDepth === remaining) { + return 1; + } + else { + return b.maxDepth - a.maxDepth; + } + }, + ); + node = - node.placeholderChildren.find((c) => c.maxDepth === remaining) || - null; + sortedPlaceholderChildren.find((c) => c.maxDepth >= remaining) || null; } else { node = node.placeholderChildren[0] || null; } @@ -138,14 +155,17 @@ function insert(ctx: RadixRouterContext, path: string, data: any) { childNode.paramName = section.slice(3 /* "**:" */) || "_"; isStaticRoute = false; } - - matchedNodes.push(childNode); node = childNode; } + + matchedNodes.push(childNode); } - for (const [depth, node] of matchedNodes.entries()) { - node.maxDepth = Math.max(matchedNodes.length - depth, node.maxDepth || 0); + for (const [depth, matchedNode] of matchedNodes.entries()) { + matchedNode.maxDepth = Math.max( + matchedNodes.length - depth, + matchedNode.maxDepth || 0, + ); } // Store whatever data was provided into the node From 12fd51516587368df5c5d3eb900cf58f9d799b64 Mon Sep 17 00:00:00 2001 From: Mohammad Bagher Abiyat Date: Sat, 11 May 2024 12:36:54 +0330 Subject: [PATCH 8/8] lint --- src/router.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/router.ts b/src/router.ts index aff1e7b..74c4d13 100644 --- a/src/router.ts +++ b/src/router.ts @@ -71,21 +71,19 @@ function lookup( (a, b) => { if (a.maxDepth === remaining && b.maxDepth === remaining) { return 0; - } - else if (a.maxDepth === remaining) { + } else if (a.maxDepth === remaining) { return -1; - } - else if (b.maxDepth === remaining) { + } else if (b.maxDepth === remaining) { return 1; - } - else { + } else { return b.maxDepth - a.maxDepth; } }, ); node = - sortedPlaceholderChildren.find((c) => c.maxDepth >= remaining) || null; + sortedPlaceholderChildren.find((c) => c.maxDepth >= remaining) || + null; } else { node = node.placeholderChildren[0] || null; }