Fix isActive logic on undefined query values

4Catalyzer · Jun 19, 2017 · 816a911 · 816a911
1 parent 2cf3da2
commit 816a911
Show file tree

Hide file tree

Showing 2 changed files with 123 additions and 7 deletions.
diff --git a/src/Matcher.js b/src/Matcher.js
@@ -145,7 +145,7 @@ export default class Matcher {
       return false;
     }
 
-    // Require that a partial match is followed by a path separator.
+    // Require that a partial match be followed by a path separator.
     const pathnameWithSeparator = pathname.slice(-1) !== '/' ?
       `${pathname}/` : pathname;
 
@@ -159,8 +159,8 @@ export default class Matcher {
     }
 
     return Object.entries(query).every(([key, value]) => (
-      Object.prototype.hasOwnProperty.call(matchQuery, key) &&
-        isEqual(matchQuery[key], value)
+      Object.prototype.hasOwnProperty.call(matchQuery, key) ?
+        isEqual(matchQuery[key], value) : value === undefined
     ));
   }
 }
diff --git a/test/Matcher.test.js b/test/Matcher.test.js
@@ -5,7 +5,7 @@ describe('Matcher', () => {
     let matcher;
 
     beforeEach(() => {
-      const routeConfig = [
+      matcher = new Matcher([
         {
           path: 'foo',
           children: [
@@ -30,9 +30,7 @@ describe('Matcher', () => {
         {
           path: 'foo/baz',
         },
-      ];
-
-      matcher = new Matcher(routeConfig);
+      ]);
     });
 
     [
@@ -166,4 +164,122 @@ describe('Matcher', () => {
       });
     });
   });
+
+  describe('#isActive', () => {
+    let matcher;
+
+    beforeEach(() => {
+      matcher = new Matcher([]);
+    });
+
+    describe('active locations', () => {
+      [
+        [
+          'path match',
+          { pathname: '/foo/bar' },
+          { pathname: '/foo/bar' },
+        ],
+        [
+          'parent path match',
+          { pathname: '/foo/bar' },
+          { pathname: '/foo' },
+        ],
+        [
+          'exact path match',
+          { pathname: '/foo/bar' },
+          { pathname: '/foo/bar' },
+          { exact: true },
+        ],
+        [
+          'null query match',
+          { pathname: '/foo', query: { foo: 'bar' } },
+          { pathname: '/foo' },
+        ],
+        [
+          'empty query match',
+          { pathname: '/foo', query: { foo: 'bar' } },
+          { pathname: '/foo', query: {} },
+        ],
+        [
+          'empty query match with explicit undefined',
+          { pathname: '/foo', query: { foo: undefined } },
+          { pathname: '/foo', query: {} },
+        ],
+        [
+          'query match',
+          { pathname: '/foo', query: { foo: 'bar' } },
+          { pathname: '/foo', query: { foo: 'bar' } },
+        ],
+        [
+          'query match with extraneous query item',
+          { pathname: '/foo', query: { foo: 'bar', bar: 'foo' } },
+          { pathname: '/foo', query: { foo: 'bar' } },
+        ],
+        [
+          'absent query match with implicit undefined',
+          { pathname: '/foo', query: {} },
+          { pathname: '/foo', query: { foo: undefined } },
+        ],
+        [
+          'absent query match with explicit undefined',
+          { pathname: '/foo', query: { foo: undefined } },
+          { pathname: '/foo', query: { foo: undefined } },
+        ],
+      ].forEach(([scenario, matchLocation, location, options]) => {
+        it(`should be active on ${scenario}`, () => {
+          expect(matcher.isActive(
+            { location: matchLocation }, location, options,
+          )).toBe(true);
+        });
+      });
+    });
+
+    describe('inactive locations', () => {
+      [
+        [
+          'path mismatch',
+          { pathname: '/bar' },
+          { pathname: '/foo' },
+        ],
+        [
+          'exact parent path match',
+          { pathname: '/foo/bar' },
+          { pathname: '/foo' },
+          { exact: true },
+        ],
+        [
+          'exact path mismatch',
+          { pathname: '/bar' },
+          { pathname: '/foo' },
+          { exact: true },
+        ],
+        [
+          'query mismatch',
+          { pathname: '/foo', query: { foo: 'foo' } },
+          { pathname: '/foo', query: { foo: 'bar' } },
+        ],
+        [
+          'missing query item',
+          { pathname: '/foo', query: {} },
+          { pathname: '/foo', query: { foo: 'bar' } },
+        ],
+        [
+          'expected query item explicitly undefined',
+          { pathname: '/foo', query: { foo: undefined } },
+          { pathname: '/foo', query: { foo: 'bar' } },
+        ],
+        [
+          'expected undefined query item present',
+          { pathname: '/foo', query: { foo: 'bar' } },
+          { pathname: '/foo', query: { foo: undefined } },
+        ],
+      ].forEach(([scenario, matchLocation, location, options]) => {
+        it(`should not be active on ${scenario}`, () => {
+          expect(matcher.isActive(
+            { location: matchLocation }, location, options,
+          )).toBe(false);
+        });
+      });
+    });
+  });
 });