fix!: last named segment should be required (#123)

unjs · Jul 24, 2024 · e19aa66 · e19aa66
1 parent c13c7ee
commit e19aa66
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 16 deletions.
diff --git a/src/operations/add.ts b/src/operations/add.ts
@@ -39,7 +39,11 @@ export function addRoute<T>(
         node.wildcard = { key: "**" };
       }
       node = node.wildcard;
-      paramsMap.push([-i, segment.split(":")[1] || "_"]);
+      paramsMap.push([
+        -i,
+        segment.split(":")[1] || "_",
+        segment.length === 2 /* no id */,
+      ]);
       break;
     }
 
@@ -49,11 +53,13 @@ export function addRoute<T>(
         node.param = { key: "*" };
       }
       node = node.param;
+      const isOptional = segment === "*";
       paramsMap.push([
         i,
-        segment === "*"
+        isOptional
           ? `_${_unnamedParamIndex++}`
           : (_getParamMatcher(segment) as string),
+        isOptional,
       ]);
       continue;
     }

diff --git a/src/operations/find.ts b/src/operations/find.ts
@@ -63,11 +63,20 @@ function _lookupTree<T>(
     if (node.param && node.param.methods) {
       const match = node.param.methods[method] || node.param.methods[""];
       if (match) {
-        return match;
+        const pMap = match[0].paramsMap;
+        if (pMap?.[pMap?.length - 1]?.[2] /* optional */) {
+          return match;
+        }
       }
     }
     if (node.wildcard && node.wildcard.methods) {
-      return node.wildcard.methods[method] || node.wildcard.methods[""];
+      const match = node.wildcard.methods[method] || node.wildcard.methods[""];
+      if (match) {
+        const pMap = match[0].paramsMap;
+        if (pMap?.[pMap?.length - 1]?.[2] /* optional */) {
+          return match;
+        }
+      }
     }
     return undefined;
   }

diff --git a/src/types.ts b/src/types.ts
@@ -3,7 +3,9 @@ export interface RouterContext<T = unknown> {
   static: Record<string, Node<T> | undefined>;
 }
 
-export type ParamsIndexMap = Array<[Index: number, name: string | RegExp]>;
+export type ParamsIndexMap = Array<
+  [Index: number, name: string | RegExp, optional: boolean]
+>;
 export type MethodData<T = unknown> = { data: T; paramsMap?: ParamsIndexMap };
 
 export interface Node<T = unknown> {

diff --git a/test/router.test.ts b/test/router.test.ts
@@ -78,13 +78,16 @@ describe("Router lookup", function () {
   describe("retrieve placeholders", function () {
     testRouter(
       [
+        "blog/*",
         "carbon/:element",
         "carbon/:element/test/:testing",
         "this/:route/has/:cool/stuff",
       ],
       (router) =>
         expect(formatTree(router.root)).toMatchInlineSnapshot(`
           "<root>
+              ├── /blog
+              │       ├── /* ┈> [GET] blog/*
               ├── /carbon
               │       ├── /* ┈> [GET] carbon/:element
               │       │       ├── /test
@@ -102,8 +105,8 @@ describe("Router lookup", function () {
             element: "test1",
           },
         },
-        "/carbon": { data: { path: "carbon/:element" } },
-        "carbon/": { data: { path: "carbon/:element" } },
+        "/carbon": undefined,
+        "carbon/": undefined,
         "carbon/test2/test/test23": {
           data: { path: "carbon/:element/test/:testing" },
           params: {
@@ -118,6 +121,9 @@ describe("Router lookup", function () {
             cool: "more",
           },
         },
+        "/blog": { data: { path: "blog/*" } },
+        "blog/": { data: { path: "blog/*" } },
+        "blog/123": { data: { path: "blog/*" } },
       },
     );
 
@@ -330,6 +336,7 @@ describe("Router lookup", function () {
           data: { path: mixedPath },
           params: { category: "test", id: "123", name: "foobar" },
         },
+        "/files/test": undefined,
       },
     );
   });
@@ -428,10 +435,7 @@ describe("Router remove", function () {
     ]);
 
     removeRoute(router, "GET", "choot");
-    expect(findRoute(router, "GET", "choot")).to.deep.equal({
-      data: { path: "choot/:choo" },
-      params: { choo: undefined },
-    });
+    expect(findRoute(router, "GET", "choot")).to.deep.equal(undefined);
     removeRoute(router, "GET", "choot/*");
     expect(findRoute(router, "GET", "choot")).to.deep.equal(undefined);
 
@@ -448,16 +452,16 @@ describe("Router remove", function () {
   });
 
   it("removes data but does not delete a node if it has children", function () {
-    const router = createRouter(["a/b", "a/b/:param1"]);
+    const router = createRouter(["a/b", "a/b/*"]);
 
     removeRoute(router, "GET", "a/b");
     expect(findRoute(router, "GET", "a/b")).to.deep.equal({
-      data: { path: "a/b/:param1" },
-      params: { param1: undefined },
+      data: { path: "a/b/*" },
+      params: { _0: undefined },
     });
     expect(findRoute(router, "GET", "a/b/c")).to.deep.equal({
-      params: { param1: "c" },
-      data: { path: "a/b/:param1" },
+      params: { _0: "c" },
+      data: { path: "a/b/*" },
     });
     removeRoute(router, "GET", "a/b/*");
     expect(findRoute(router, "GET", "a/b")).to.deep.equal(undefined);