Skip to content

Commit

Permalink
feat: add ignoreQueryParams argument in router.isActive(), default to…
Browse files Browse the repository at this point in the history
… true

Fixes router5/react-router5#8
BREAKING CHANGE: isActive will now ignore query parameters by default.
  • Loading branch information
troch committed Oct 17, 2015
1 parent cbc2b83 commit c10dd0b
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 17 deletions.
33 changes: 22 additions & 11 deletions modules/router5.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,18 +237,21 @@ class Router5 {
* Whether or not the given route name with specified params is active.
* @param {String} name The route name
* @param {Object} [params={}] The route parameters
* @param {Boolean} [equality=false] If set to false (default), isActive will return true
* if the provided route name and params are descendants
* of the active state.
* @return {Boolean} Whether nor not the route is active
*/
isActive(name, params = {}, strictEquality = false) {
* @param {Boolean} [strictEquality=false] If set to false (default), isActive will return true
* if the provided route name and params are descendants
* of the active state.
* @param {Boolean} [ignoreQueryParams=true] Whether or not to ignore URL query parameters when
* comparing the two states together.
* query parameters when comparing two states together.
* @return {Boolean} Whether nor not the route is active
*/
isActive(name, params = {}, strictEquality = false, ignoreQueryParams = true) {
let activeState = this.getState()

if (!activeState) return false

if (strictEquality || activeState.name === name) {
return this.areStatesEqual(makeState(name, params), activeState)
return this.areStatesEqual(makeState(name, params), activeState, ignoreQueryParams)
}

return this.areStatesDescendants(makeState(name, params), activeState)
Expand All @@ -257,10 +260,18 @@ class Router5 {
/**
* @private
*/
areStatesEqual(state1, state2) {
return state1.name === state2.name &&
Object.keys(state1.params).length === Object.keys(state2.params).length &&
Object.keys(state1.params).every(p => state1.params[p] === state2.params[p])
areStatesEqual(state1, state2, ignoreQueryParams = true) {
if (state1.name !== state2.name) return false;

const getUrlParams = name => this.rootNode
.getSegmentsByName(name)
.map(segment => segment.parser[ignoreQueryParams ? 'urlParams' : 'params'])
.reduce((params, p) => params.concat(p), []);

const state1Params = getUrlParams(state1.name);
const state2Params = getUrlParams(state2.name);

return state1Params.length === state2Params.length && state1Params.every(p => state1.params[p] === state2.params[p]);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion tests/_create-router.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
]);

var sectionRoute = new RouteNode('section', '/:section<section[\\d]+>', [
new RouteNode('view', '/view/:id')
new RouteNode('view', '/view/:id'),
new RouteNode('query', '/query?param2&param3')
]);

function createRouter(base, useHash, hashPrefix) {
Expand Down
10 changes: 5 additions & 5 deletions tests/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('router5', function () {
testRouter(false);

// With hash
// testRouter(true);
testRouter(true);
});

function testRouter(useHash) {
Expand Down Expand Up @@ -434,11 +434,11 @@ function testRouter(useHash) {
expect(router.isActive('users')).toBe(true);
expect(router.isActive('users', {}, true)).toBe(false);

router.navigate('section.view', {section: 'section1', id: 12});
router.navigate('section.query', {section: 'section1'});
expect(router.isActive('section', {section: 'section1'})).toBe(true);
expect(router.isActive('section.view', {section: 'section1', id: 12})).toBe(true);
expect(router.isActive('section.view', {section: 'section2', id: 12})).toBe(false);
expect(router.isActive('section.view', {section: 'section1', id: 123})).toBe(false);
expect(router.isActive('section.query', {section: 'section1', param1: '123'})).toBe(true);
expect(router.isActive('section.query', {section: 'section2'})).toBe(false);
expect(router.isActive('section.query', {section: 'section1', param2: '123'}, false, false)).toBe(false);
expect(router.isActive('users.view', {id: 123})).toBe(false);
});

Expand Down

3 comments on commit c10dd0b

@vojtech-dobes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for awesomely fast response :). My team and me really appreciate it.

I've spotted there is undesired consequence of this change. When I use router.navigate from foo and no params to foo and ?bar=1, it doesn't work because... query parameters are ignored by default and router.navigate uses areStatesEqual :).

@troch
Copy link
Member Author

@troch troch commented on c10dd0b Oct 19, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vojtech-dobes patched (v0.9.2) 👍

@vojtech-dobes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Please sign in to comment.