fix: ordering of siblings routes for matching

Author: troch

modules/RouteNode.js

@@ -56,29 +56,33 @@ export default class RouteNode {
             this.children.push(route);
             // Push greedy spats to the bottom of the pile
             this.children.sort((left, right) => {
-                const leftPath = left.path.split('?')[0];
-                const rightPath = right.path.split('?')[0];
+                const leftPath = left.path.split('?')[0].replace(/(.+)\/$/, '$1');
+                const rightPath = right.path.split('?')[0].replace(/(.+)\/$/, '$1');
                 // '/' last
                 if (leftPath === '/') return 1;
                 if (rightPath === '/') return -1;
-                let leftHasParams = left.parser.hasUrlParams || left.parser.hasSpatParam;
-                let rightHasParams = right.parser.hasUrlParams || right.parser.hasSpatParam;
-                // No params first, sort by length descending
-                if (!leftHasParams && !rightHasParams) {
-                    return leftPath && rightPath ? (leftPath.length < rightPath.length ? 1 : -1) : 0;
-                }
-                // Params last
-                if (leftHasParams && !rightHasParams) return 1;
-                if (!leftHasParams && rightHasParams) return -1;
                 // Spat params last
-                if (!left.parser.hasSpatParam && right.parser.hasSpatParam) return -1;
-                if (!right.parser.hasSpatParam && left.parser.hasSpatParam) return 1;
-                // Sort by number of segments descending
-                let leftSegments = (leftPath.match(/\//g) || []).length;
-                let rightSegments = (rightPath.match(/\//g) || []).length;
+                if (left.parser.hasSpatParam) return 1;
+                if (right.parser.hasSpatParam) return -1;
+                // No spat, number of segments (less segments last)
+                const leftSegments = (leftPath.match(/\//g) || []).length;
+                const rightSegments = (rightPath.match(/\//g) || []).length;
                 if (leftSegments < rightSegments) return 1;
+                if (leftSegments > rightSegments) return -1;
+                // Same number of segments, number of URL params ascending
+                const leftParamsCount = left.parser.urlParams.length;
+                const rightParamsCount = right.parser.urlParams.length;
+                if (leftParamsCount < rightParamsCount) return -1;
+                if (leftParamsCount > rightParamsCount) return 1;
+                // Same number of segments and params, last segment length descending
+                const leftParamLength = (leftPath.split('/').slice(-1)[0] || '').length;
+                const rightParamLength = (rightPath.split('/').slice(-1)[0] || '').length;
+                if (leftParamLength < rightParamLength) return 1;
+                if (leftParamLength > rightParamLength) return -1;
+                // Same last segment length, preserve definition order
                 return 0;
             });
+            console.log(this.children.map(c => c.path));
         } else {
             // Locate parent node
             let segments = this.getSegmentsByName(names.slice(0, -1).join('.'));
@@ -176,7 +180,6 @@ export default class RouteNode {
 
         const matched = matchChildren(startingNodes, path, segments);
         if (matched && matched.length === 1 && matched[0].name === '') return null;
-
         return matched;
     }
 

package.json

@@ -54,6 +54,7 @@
     "yargs": "^3.31.0"
   },
   "dependencies": {
-    "path-parser": "~1.0.2"
+    "path-parser": "~1.0.2",
+    "search-params": "~1.1.0"
   }
 }

test/main.js

@@ -1,5 +1,3 @@
-'use strict';
-
 import path from 'path';
 import pkg from '../package.json';
 import RouteNode from '../modules/RouteNode';
@@ -272,7 +270,7 @@ describe('RouteNode', function () {
 
         withoutMeta(usersNode.matchPath('/users/view/1')).should.eql({name: 'users.view', params: {id: '1'}});
         withoutMeta(usersNode.matchPath('/users/list')).should.eql({name: 'users.list', params: {}});
-    })
+    });
 
     it('should be able to add deep nodes', function () {
         var rootNode = new RouteNode('', '')
@@ -286,17 +284,28 @@ describe('RouteNode', function () {
 
     it('should sort paths by length', function () {
         var rootNode = new RouteNode('', '')
+            .addNode('personList', '/persons/')
+            .addNode('personDetail', '/persons/:personId')
             .addNode('section', '/section/:id?a')
             .addNode('index', '/?queryparamOfexceptionalLength')
             .addNode('id', '/:id?rrrr')
             .addNode('abo', '/abo')
-            .addNode('about', '/about?hello');
+            .addNode('about', '/about?hello')
+            .addNode('users', '/users-tab')
+            .addNode('user', '/users/:id')
+            .addNode('postNew', '/blogs/:blogId/posts/new')
+            .addNode('postDetail', '/blogs/:blogId/posts/:postId');
 
         withoutMeta(rootNode.matchPath('/')).should.eql({name: 'index', params: {}});
         withoutMeta(rootNode.matchPath('/abo')).should.eql({name: 'abo', params: {}});
         withoutMeta(rootNode.matchPath('/about')).should.eql({name: 'about', params: {}});
         withoutMeta(rootNode.matchPath('/abc')).should.eql({name: 'id', params: {id: 'abc'}});
         withoutMeta(rootNode.matchPath('/section/abc')).should.eql({name: 'section', params: {id: 'abc'}});
+        withoutMeta(rootNode.matchPath('/persons/jwoudenberg')).should.eql({name: 'personDetail', params: {personId: 'jwoudenberg'}});
+        withoutMeta(rootNode.matchPath('/users-tab')).should.eql({name: 'users', params: {}});
+        withoutMeta(rootNode.matchPath('/users/thomas')).should.eql({name: 'user', params: {id: 'thomas'}});
+        withoutMeta(rootNode.matchPath('/blogs/123/posts/new')).should.eql({name: 'postNew', params: {blogId: '123'}});
+        withoutMeta(rootNode.matchPath('/blogs/123/posts/456')).should.eql({name: 'postDetail', params: {blogId: '123', postId: '456'}});
     });
 
     it('should match paths with optional trailing slashes', function () {
@@ -317,7 +326,7 @@ describe('RouteNode', function () {
         should.not.exists(rootNode.matchPath('/users/list//', { trailingSlash: true }));
     });
 
-    it.only('should support query parameters with square brackets', function () {
+    it('should support query parameters with square brackets', function () {
         var node = new RouteNode('', '', [
             new RouteNode('route', '/route?arr[]', [
                 new RouteNode('deep', '/deep?arr2[]')