Skip to content

Commit

Permalink
fix($state.go): param inheritance shouldn't inherit from siblings
Browse files Browse the repository at this point in the history
Due to a bug in the ancestors() function, we were treating any states at the same depth from root as "ancestors". This means siblings were inheriting parameters from each other. Interestingly ui-sref generated the correct links, but the click handler then broke the link.

Unfortunately this is a breaking change if someone depends on the broken behavior of inheriting all the params on sibling state transitions. The fix for these folks is mostly simple: create a common parent state that contains parameters that need to be shared. Unfortunately it can introduce quite a bit of churn in the codebase.
  • Loading branch information
Mike Kaplinskiy committed Dec 27, 2013
1 parent e3d5647 commit aea872e
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 3 deletions.
3 changes: 1 addition & 2 deletions src/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ function ancestors(first, second) {
var path = [];

for (var n in first.path) {
if (first.path[n] === "") continue;
if (!second.path[n]) break;
if (first.path[n] !== second.path[n]) break;
path.push(first.path[n]);
}
return path;
Expand Down
34 changes: 33 additions & 1 deletion test/stateSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,13 @@ describe('state', function () {
})

.state('first', { url: '^/first/subpath' })
.state('second', { url: '^/second' });
.state('second', { url: '^/second' })

// State param inheritance tests. param1 is inherited by sub1 & sub2;
// param2 should not be transferred (unless explicitly set).
.state('root', { url: '^/root?param1' })
.state('root.sub1', {url: '/1?param2' })
.state('root.sub2', {url: '/2?param2' });

$provide.value('AppInjectable', AppInjectable);
}));
Expand Down Expand Up @@ -669,6 +675,9 @@ describe('state', function () {
'home.redirect',
'resolveFail',
'resolveTimeout',
'root',
'root.sub1',
'root.sub2',
'second'
];
expect(list.map(function(state) { return state.name; })).toEqual(names);
Expand Down Expand Up @@ -802,6 +811,29 @@ describe('state', function () {
}));
});

describe('substate and stateParams inheritance', function() {
it('should inherit the parent param', inject(function ($state, $stateParams, $q) {
initStateTo($state.get('root'), {param1: 1});
$state.go('root.sub1', {param2: 2});
$q.flush();
expect($state.current.name).toEqual('root.sub1');
expect($stateParams).toEqual({param1: '1', param2: '2'});
}));

it('should not inherit siblings\' states', inject(function ($state, $stateParams, $q) {
initStateTo($state.get('root'), {param1: 1});
$state.go('root.sub1', {param2: 2});
$q.flush();
expect($state.current.name).toEqual('root.sub1');

$state.go('root.sub2');
$q.flush();
expect($state.current.name).toEqual('root.sub2');

expect($stateParams).toEqual({param1: '1', param2: null});
}));
});

describe('html5Mode compatibility', function() {

it('should generate non-hashbang URLs in HTML5 mode', inject(function ($state) {
Expand Down

0 comments on commit aea872e

Please sign in to comment.