Merge pull request visionmedia#22 from ovaillancourt/master

Fix for page.js not matching routes containing query strings.
cjblomqvist · Jul 5, 2012 · 9b60993 · 9b60993
2 parents 9a4cc7a + 14315c2
commit 9b60993
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 9 deletions.
diff --git a/build/page.js b/build/page.js
@@ -88,7 +88,7 @@
     if (false !== options.popstate) addEventListener('popstate', onpopstate, false);
     if (false !== options.click) addEventListener('click', onclick, false);
     if (!dispatch) return;
-    page.replace(location.pathname, null, true, dispatch);
+    page.replace(location.pathname + location.search, null, true, dispatch);
   };
 
   /**
@@ -264,8 +264,10 @@
 
   Route.prototype.match = function(path, params){
     var keys = this.keys
-      , m = this.regexp.exec(path);
-
+      , qsIndex = path.indexOf('?')
+      , pathname = ~qsIndex ? path.slice(0, qsIndex) : path
+      , m = this.regexp.exec(pathname);
+
     if (!m) return false;
 
     for (var i = 1, len = m.length; i < len; ++i) {
@@ -348,7 +350,7 @@
     while (el && 'A' != el.nodeName) el = el.parentNode;
     if (!el || 'A' != el.nodeName) return;
     var href = el.href;
-    var path = el.pathname;
+    var path = el.pathname + el.search;
     if (el.hash) return;
     if (!sameOrigin(href)) return;
     var orig = path;

diff --git a/build/page.min.js b/build/page.min.js
diff --git a/lib/page.js b/lib/page.js
@@ -88,7 +88,7 @@
     if (false !== options.popstate) addEventListener('popstate', onpopstate, false);
     if (false !== options.click) addEventListener('click', onclick, false);
     if (!dispatch) return;
-    page.replace(location.pathname, null, true, dispatch);
+    page.replace(location.pathname + location.search, null, true, dispatch);
   };
 
   /**
@@ -264,8 +264,10 @@
 
   Route.prototype.match = function(path, params){
     var keys = this.keys
-      , m = this.regexp.exec(path);
-
+      , qsIndex = path.indexOf('?')
+      , pathname = ~qsIndex ? path.slice(0, qsIndex) : path
+      , m = this.regexp.exec(pathname);
+
     if (!m) return false;
 
     for (var i = 1, len = m.length; i < len; ++i) {
@@ -348,7 +350,7 @@
     while (el && 'A' != el.nodeName) el = el.parentNode;
     if (!el || 'A' != el.nodeName) return;
     var href = el.href;
-    var path = el.pathname;
+    var path = el.pathname + el.search;
     if (el.hash) return;
     if (!sameOrigin(href)) return;
     var orig = path;

diff --git a/test/tests.js b/test/tests.js
@@ -15,6 +15,42 @@ describe('page', function(){
     })
   })
 
+  describe('when matching routes', function(){
+    describe('when the path contains a query string', function(){
+      it('should not consider the query string when matching', function(done){
+        page('/qs', function(ctx){
+          done();
+        });
+
+        page('/qs?test=true');
+      })
+
+      it('should not consider the query string when matching a route ending with a named param', function(done){
+        page('/qs/:testParam', function(ctx){
+          done();
+        });
+
+        page('/qs/test?test=true');
+      })
+
+      it('should not consider the query string when matching a route ending with a named wildcard param', function(done){
+        page('/qs/named/wildcard/:testParam(*)', function(ctx){
+          done();
+        });
+
+        page('/qs/named/wildcard/something/else?test=true');
+      })
+
+      it('should not consider the query string when matching a route ending with a wildcard', function(done){
+        page('/qs/wildcard/end/*', function(ctx){
+          done();
+        });
+
+        page('/qs/wildcard/end/with/something/here?test=true');
+      });
+    })
+  })
+
   describe('when the route matches', function(){
     it('should invoke the callback', function(done){
       page('/user/:name', function(ctx){
@@ -32,6 +68,28 @@ describe('page', function(){
 
       page('/blog/post/something');
     })
+
+    describe('when the match has a query string', function(){
+
+      it('should not include the query string inside ctx.params for named param values', function(done){
+        page('/query/string/:name', function(ctx){
+          expect(ctx.params.name).to.equal('something');
+          done();
+        })
+
+        page('/query/string/something?test=true');
+      })
+
+      it('should not include the query string inside ctx.params for named wildcard params', function(done){
+        page('/query/string/wildcard/:name(*)', function(ctx){
+          expect(ctx.params.name).to.equal('something');
+          done();
+        })
+
+        page('/query/string/wildcard/something?test=true');
+      })
+    })
+
   })
 
   describe('when next() is called', function(){