From f6f454ed42b2043194d31887d41b99a5d149a0aa Mon Sep 17 00:00:00 2001 From: Yusuke Wada Date: Sun, 24 Jul 2022 11:03:04 +0900 Subject: [PATCH] fix(trie-router): fix the rule for capturing named parameter (#419) Close #418 --- deno_dist/router/trie-router/node.ts | 34 ++++++++++++++++----------- src/router/trie-router/node.test.ts | 18 ++++++++++++++ src/router/trie-router/node.ts | 34 ++++++++++++++++----------- src/router/trie-router/router.test.ts | 16 +++++++++++++ 4 files changed, 74 insertions(+), 28 deletions(-) diff --git a/deno_dist/router/trie-router/node.ts b/deno_dist/router/trie-router/node.ts index 62da1027..f5002759 100644 --- a/deno_dist/router/trie-router/node.ts +++ b/deno_dist/router/trie-router/node.ts @@ -135,9 +135,23 @@ export class Node { const part: string = parts[i] const isLast = i === len - 1 const tempNodes: Node[] = [] + let matched = false for (let j = 0, len2 = curNodes.length; j < len2; j++) { const node = curNodes[j] + const nextNode = node.children[part] + + if (nextNode) { + if (isLast === true) { + // '/hello/*' => match '/hello' + if (nextNode.children['*']) { + handlerSets.push(...this.getHandlerSets(nextNode.children['*'], method, true)) + } + handlerSets.push(...this.getHandlerSets(nextNode, method)) + matched = true + } + tempNodes.push(nextNode) + } for (let k = 0, len3 = node.patterns.length; k < len3; k++) { const pattern = node.patterns[k] @@ -165,24 +179,16 @@ export class Node { } tempNodes.push(node.children[key]) } - if (typeof name === 'string') { + + // '/book/a' => not-slug + // '/book/:slug' => slug + // GET /book/a ~> no-slug, param['slug'] => undefined + // GET /book/foo ~> slug, param['slug'] => foo + if (typeof name === 'string' && !matched) { params[name] = part } } } - - const nextNode = node.children[part] - - if (nextNode) { - if (isLast === true) { - // '/hello/*' => match '/hello' - if (nextNode.children['*']) { - handlerSets.push(...this.getHandlerSets(nextNode.children['*'], method, true)) - } - handlerSets.push(...this.getHandlerSets(nextNode, method)) - } - tempNodes.push(nextNode) - } } curNodes = tempNodes diff --git a/src/router/trie-router/node.test.ts b/src/router/trie-router/node.test.ts index f4440a14..fedd0ed4 100644 --- a/src/router/trie-router/node.test.ts +++ b/src/router/trie-router/node.test.ts @@ -527,3 +527,21 @@ describe('star', () => { expect(res?.handlers).toEqual(['/*', '*', '/x', '/x/*']) }) }) + +describe('Routing order With named parameters', () => { + const node = new Node() + node.insert('get', '/book/a', 'no-slug') + node.insert('get', '/book/:slug', 'slug') + it('/book/a', () => { + const res = node.search('get', '/book/a') + expect(res).not.toBeNull() + expect(res?.handlers).toEqual(['no-slug', 'slug']) + expect(res?.params['slug']).toBeUndefined() + }) + it('/book/foo', () => { + const res = node.search('get', '/book/foo') + expect(res).not.toBeNull() + expect(res?.handlers).toEqual(['slug']) + expect(res?.params['slug']).toBe('foo') + }) +}) diff --git a/src/router/trie-router/node.ts b/src/router/trie-router/node.ts index 75e2b127..2e6e0e53 100644 --- a/src/router/trie-router/node.ts +++ b/src/router/trie-router/node.ts @@ -135,9 +135,23 @@ export class Node { const part: string = parts[i] const isLast = i === len - 1 const tempNodes: Node[] = [] + let matched = false for (let j = 0, len2 = curNodes.length; j < len2; j++) { const node = curNodes[j] + const nextNode = node.children[part] + + if (nextNode) { + if (isLast === true) { + // '/hello/*' => match '/hello' + if (nextNode.children['*']) { + handlerSets.push(...this.getHandlerSets(nextNode.children['*'], method, true)) + } + handlerSets.push(...this.getHandlerSets(nextNode, method)) + matched = true + } + tempNodes.push(nextNode) + } for (let k = 0, len3 = node.patterns.length; k < len3; k++) { const pattern = node.patterns[k] @@ -165,24 +179,16 @@ export class Node { } tempNodes.push(node.children[key]) } - if (typeof name === 'string') { + + // '/book/a' => not-slug + // '/book/:slug' => slug + // GET /book/a ~> no-slug, param['slug'] => undefined + // GET /book/foo ~> slug, param['slug'] => foo + if (typeof name === 'string' && !matched) { params[name] = part } } } - - const nextNode = node.children[part] - - if (nextNode) { - if (isLast === true) { - // '/hello/*' => match '/hello' - if (nextNode.children['*']) { - handlerSets.push(...this.getHandlerSets(nextNode.children['*'], method, true)) - } - handlerSets.push(...this.getHandlerSets(nextNode, method)) - } - tempNodes.push(nextNode) - } } curNodes = tempNodes diff --git a/src/router/trie-router/router.test.ts b/src/router/trie-router/router.test.ts index a7337073..d94b576f 100644 --- a/src/router/trie-router/router.test.ts +++ b/src/router/trie-router/router.test.ts @@ -129,3 +129,19 @@ describe('page', () => { expect(res?.handlers).toEqual(['page', 'fallback']) }) }) + +describe('routing order with named parameters', () => { + const router = new TrieRouter() + router.add('GET', '/book/a', 'no-slug') + router.add('GET', '/book/:slug', 'slug') + it('GET /book/a', async () => { + const res = router.match('GET', '/book/a') + expect(res?.handlers).toEqual(['no-slug', 'slug']) + expect(res?.params['slug']).toBeUndefined() + }) + it('GET /book/foo', async () => { + const res = router.match('GET', '/book/foo') + expect(res?.handlers).toEqual(['slug']) + expect(res?.params['slug']).toBe('foo') + }) +})