Skip to content

Commit aea872e

Browse files
author
Mike Kaplinskiy
committed
fix($state.go): param inheritance shouldn't inherit from siblings
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.
1 parent e3d5647 commit aea872e

File tree

2 files changed

+34
-3
lines changed

2 files changed

+34
-3
lines changed

src/common.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@ function ancestors(first, second) {
3737
var path = [];
3838

3939
for (var n in first.path) {
40-
if (first.path[n] === "") continue;
41-
if (!second.path[n]) break;
40+
if (first.path[n] !== second.path[n]) break;
4241
path.push(first.path[n]);
4342
}
4443
return path;

test/stateSpec.js

+33-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,13 @@ describe('state', function () {
9393
})
9494

9595
.state('first', { url: '^/first/subpath' })
96-
.state('second', { url: '^/second' });
96+
.state('second', { url: '^/second' })
97+
98+
// State param inheritance tests. param1 is inherited by sub1 & sub2;
99+
// param2 should not be transferred (unless explicitly set).
100+
.state('root', { url: '^/root?param1' })
101+
.state('root.sub1', {url: '/1?param2' })
102+
.state('root.sub2', {url: '/2?param2' });
97103

98104
$provide.value('AppInjectable', AppInjectable);
99105
}));
@@ -669,6 +675,9 @@ describe('state', function () {
669675
'home.redirect',
670676
'resolveFail',
671677
'resolveTimeout',
678+
'root',
679+
'root.sub1',
680+
'root.sub2',
672681
'second'
673682
];
674683
expect(list.map(function(state) { return state.name; })).toEqual(names);
@@ -802,6 +811,29 @@ describe('state', function () {
802811
}));
803812
});
804813

814+
describe('substate and stateParams inheritance', function() {
815+
it('should inherit the parent param', inject(function ($state, $stateParams, $q) {
816+
initStateTo($state.get('root'), {param1: 1});
817+
$state.go('root.sub1', {param2: 2});
818+
$q.flush();
819+
expect($state.current.name).toEqual('root.sub1');
820+
expect($stateParams).toEqual({param1: '1', param2: '2'});
821+
}));
822+
823+
it('should not inherit siblings\' states', inject(function ($state, $stateParams, $q) {
824+
initStateTo($state.get('root'), {param1: 1});
825+
$state.go('root.sub1', {param2: 2});
826+
$q.flush();
827+
expect($state.current.name).toEqual('root.sub1');
828+
829+
$state.go('root.sub2');
830+
$q.flush();
831+
expect($state.current.name).toEqual('root.sub2');
832+
833+
expect($stateParams).toEqual({param1: '1', param2: null});
834+
}));
835+
});
836+
805837
describe('html5Mode compatibility', function() {
806838

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

0 commit comments

Comments
 (0)