diff --git a/src/state.js b/src/state.js index 62d504ba0..d33f71d09 100644 --- a/src/state.js +++ b/src/state.js @@ -954,7 +954,7 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { * have not changed, aka a reload of the same state. It differs from reloadOnSearch because you'd * use this when you want to force a reload when *everything* is the same, including search params. * if String, then will reload the state with the name given in reload, and any children. - * if Object, then a stateObj is expected, will reload the state found in stateObj, and any chhildren. + * if Object, then a stateObj is expected, will reload the state found in stateObj, and any children. * * @returns {promise} A promise representing the state of the new transition. See * {@link ui.router.state.$state#methods_go $state.go}. @@ -1002,7 +1002,7 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { // Starting from the root of the path, keep all levels that haven't changed var keep = 0, state = toPath[keep], locals = root.locals, toLocals = []; - var skipTriggerReloadCheck = false; + var allowSkipReloadCheck = true; if (!options.reload) { while (state && state === fromPath[keep] && state.ownParams.$$equals(toParams, fromParams)) { @@ -1020,7 +1020,9 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { throw new Error("No such reload state '" + (isString(options.reload) ? options.reload : options.reload.name) + "'"); } - skipTriggerReloadCheck = true; + // Question for PR review: Why do we not reset this for options.reload === true?! + // (shouldSkipReload had an embedded check for options.reload but it wouldn't need it if we did this) + allowSkipReloadCheck = false; while (state && state === fromPath[keep] && state !== reloadState) { locals = toLocals[keep] = state.locals; @@ -1034,7 +1036,7 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { // TODO: We may not want to bump 'transition' if we're called from a location change // that we've initiated ourselves, because we might accidentally abort a legitimate // transition initiated from code? - if (!skipTriggerReloadCheck && shouldTriggerReload(to, from, locals, options)) { + if (allowSkipReloadCheck && shouldSkipReload(to, toParams, from, fromParams, locals, options)) { if (to.self.reloadOnSearch !== false) $urlRouter.update(); $state.transition = null; return $q.when($state.current); @@ -1429,11 +1431,43 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { return $state; } - function shouldTriggerReload(to, from, locals, options) { - if (to === from && ((locals === from.locals && !options.reload) || (to.self.reloadOnSearch === false))) { + function shouldSkipReload(to, toParams, from, fromParams, locals, options) { + // If reload was not explicitly requested + // and we're transitioning to the same state we're already in + // and the locals didn't change + // or they changed in a way that doesn't merit reloading + // (reloadOnParams:false, or reloadOnSearch.false and only search params changed) + // Then skip the reload. + // NB: If we skip the reload, $stateParams does not get updated. So people have to manage + // $location.search() entirely manually, not via $stateParams, if using reloadOnSearch: false. + if (!options.reload && to === from && (locals === from.locals || (to.self.reloadOnSearch === false && onlySearchParamsChanged(from, fromParams, to, toParams)))) { return true; } } + + function noteNonSearchParams(params, nonSearchParamsHash) { + for (var key in params) { + if (key.indexOf('$$') !== 0 && params[key].location !== 'search') { + nonSearchParamsHash[key] = true; + } + } + } + + function onlySearchParamsChanged(from, fromParams, to, toParams) { + // Identify whether all the parameters that differ between `fromParams` and `toParams` were search params. + // Return true if the only differences were in search params, false if there were differences in other params. + // Algorithm: build a set of keys of params that are not search params. + // Walk that set, comparing the actual values between the fromParams and toParams. + var nonSearchParams = {}; + noteNonSearchParams(from.params, nonSearchParams); + noteNonSearchParams(to.params, nonSearchParams); + for (var key in nonSearchParams) { + if (fromParams[key] !== toParams[key]) { + return false; + } + } + return true; + } } angular.module('ui.router.state') diff --git a/test/stateSpec.js b/test/stateSpec.js index f2aa4dc46..7b3a4f36d 100644 --- a/test/stateSpec.js +++ b/test/stateSpec.js @@ -29,6 +29,7 @@ describe('state', function () { HH = { parent: H }, HHH = {parent: HH, data: {propA: 'overriddenA', propC: 'propC'} }, RS = { url: '^/search?term', reloadOnSearch: false }, + RSP = { url: '^/:doReload/search?term', reloadOnSearch: false }, OPT = { url: '/opt/:param', params: { param: "100" } }, OPT2 = { url: '/opt2/:param2/:param3', params: { param3: "300", param4: "400" } }, AppInjectable = {}; @@ -55,6 +56,7 @@ describe('state', function () { .state('OPT', OPT) .state('OPT.OPT2', OPT2) .state('RS', RS) + .state('RSP', RSP) .state('home', { url: "/" }) .state('home.item', { url: "front/:id" }) @@ -212,7 +214,20 @@ describe('state', function () { }); $q.flush(); expect($location.search()).toEqual({term: 'hello'}); - expect(called).toBeFalsy(); + expect(called).toBeFalsy(); + })); + + it('does trigger state change for path params even if reloadOnSearch is false', inject(function ($state, $q, $location, $rootScope){ + initStateTo(RSP, { doReload: 'foo' }); + expect($state.params.doReload).toEqual('foo'); + var called; + $rootScope.$on('$stateChangeStart', function (ev, to, toParams, from, fromParams) { + called = true + }); + $state.transitionTo(RSP, { doReload: 'bar' }); + $q.flush(); + expect($state.params.doReload).toEqual('bar'); + expect(called).toBeTruthy(); })); it('ignores non-applicable state parameters', inject(function ($state, $q) { @@ -940,6 +955,7 @@ describe('state', function () { 'OPT', 'OPT.OPT2', 'RS', + 'RSP', 'about', 'about.person', 'about.person.item',