From 5314c87a16bc5065b28a06294bcc56b3a1d9c8d6 Mon Sep 17 00:00:00 2001 From: scalvert Date: Mon, 13 Mar 2017 10:42:33 -0700 Subject: [PATCH 1/2] Adding isActive --- packages/ember-routing/lib/services/router.js | 64 +++++ .../ember-routing/lib/system/router_state.js | 1 + .../router_service_test/isActive_test.js | 243 ++++++++++++++++++ .../router_service_test/urlFor_test.js | 27 +- .../lib/test-cases/router.js | 6 + 5 files changed, 324 insertions(+), 17 deletions(-) create mode 100644 packages/ember/tests/routing/router_service_test/isActive_test.js diff --git a/packages/ember-routing/lib/services/router.js b/packages/ember-routing/lib/services/router.js index e4c385e157f..e9894c99631 100644 --- a/packages/ember-routing/lib/services/router.js +++ b/packages/ember-routing/lib/services/router.js @@ -7,6 +7,24 @@ import { Service, readOnly } from 'ember-runtime'; +import { + get, + isEmpty +} from 'ember-metal'; +import { assign } from 'ember-utils'; +import RouterDSL from '../system/dsl'; + + +function shallowEqual(a, b) { + let k; + for (k in a) { + if (a.hasOwnProperty(k) && a[k] !== b[k]) { return false; } + } + for (k in b) { + if (b.hasOwnProperty(k) && a[k] !== b[k]) { return false; } + } + return true; +} /** The Router service is the public API that provides component/view layer @@ -19,6 +37,7 @@ import { const RouterService = Service.extend({ currentRouteName: readOnly('router.currentRouteName'), currentURL: readOnly('router.currentURL'), + currentState: readOnly('router.currentState'), location: readOnly('router.location'), rootURL: readOnly('router.rootURL'), router: null, @@ -79,6 +98,51 @@ const RouterService = Service.extend({ */ urlFor(/* routeName, ...models, options */) { return this.router.generate(...arguments); + }, + + /** + Determines whether a route is active. + + @method urlFor + @param {String} routeName the name of the route + @param {...Object} models the model(s) or identifier(s) to be used while + transitioning to the route. + @param {Object} [options] optional hash with a queryParams property + containing a mapping of query parameters + @return {String} the string representing the generated URL + @public + */ + isActive(/* routeName, ...models, options */) { + if (!this.router.isActive(...arguments)) { return false; } +debugger; + let { routeName, models, queryParams } = this._extractArguments(...arguments); + let emptyQueryParams = Object.keys(queryParams).length; + + if (!emptyQueryParams) { + let visibleQueryParams = {}; + assign(visibleQueryParams, queryParams); + + this.router._prepareQueryParams(routeName, models, visibleQueryParams); + return shallowEqual(visibleQueryParams, queryParams); + } + + return true; + }, + + _extractArguments(...args) { + let routeName; + let models; + let possibleQueryParams = args[args.length - 1]; + let queryParams = {}; + + if (possibleQueryParams && possibleQueryParams.hasOwnProperty('queryParams')) { + queryParams = args.pop().queryParams; + } + + routeName = args.shift(); + models = args; + + return { routeName, models, queryParams }; } }); diff --git a/packages/ember-routing/lib/system/router_state.js b/packages/ember-routing/lib/system/router_state.js index 446136de6b2..09a572348d2 100644 --- a/packages/ember-routing/lib/system/router_state.js +++ b/packages/ember-routing/lib/system/router_state.js @@ -8,6 +8,7 @@ export default EmberObject.extend({ routerJsState: null, isActiveIntent(routeName, models, queryParams, queryParamsMustMatch) { + debugger; let state = this.routerJsState; if (!this.routerJs.isActiveIntent(routeName, models, null, state)) { return false; } diff --git a/packages/ember/tests/routing/router_service_test/isActive_test.js b/packages/ember/tests/routing/router_service_test/isActive_test.js new file mode 100644 index 00000000000..aaa91f9cf0a --- /dev/null +++ b/packages/ember/tests/routing/router_service_test/isActive_test.js @@ -0,0 +1,243 @@ +import { + Controller, + inject, + String +} from 'ember-runtime'; +import { Component } from 'ember-glimmer'; +import { Route, NoneLocation } from 'ember-routing'; +import { + get, + set +} from 'ember-metal'; +import { + RouterTestCase, + moduleFor +} from 'internal-test-helpers'; + +import { EMBER_ROUTING_ROUTER_SERVICE } from 'ember/features'; + +if (EMBER_ROUTING_ROUTER_SERVICE) { + moduleFor('Router Service - isActive', class extends RouterTestCase { + ['@test RouterService#isActive returns true for simple route'](assert) { + assert.expect(1); + + return this.visit('/') + .then(() => { + return this.routerService.transitionTo('parent.child'); + }) + .then(() => { + return this.routerService.transitionTo('parent.sister'); + }) + .then(() => { + assert.ok(this.routerService.isActive('parent.sister')); + }); + } + + ['@test RouterService#isActive returns true for simple route with dynamic segments'](assert) { + assert.expect(1); + + let dynamicModel = { id: 1 }; + + return this.visit('/') + .then(() => { + return this.routerService.transitionTo('dynamic', dynamicModel); + }) + .then(() => { + assert.ok(this.routerService.isActive('dynamic', dynamicModel)); + }); + } + + ['@test RouterService#urlFor returns URL for simple route with basic query params'](assert) { + assert.expect(2); + + let queryParams = this.buildQueryParams({ sort: 'ASC' }); + + this.registerController('parent.child', Controller.extend({ + queryParams: ['sort'], + sort: 'ASC' + }) + ); + debugger; + return this.visit('/') + .then(() => { + return this.routerService.transitionTo('parent.child', queryParams); + }) + .then(() => { + assert.ok(this.routerService.isActive('parent.child', queryParams)); + assert.notOk(this.routerService.isActive('parent.child', this.buildQueryParams({ sort: 'DESC' }))); + }); + } + + ['@test RouterService#urlFor returns URL for simple route with array as query params'](assert) { + assert.expect(1); + + let queryParams = this.buildQueryParams({ sort: ['ascending'] }); + + return this.visit('/') + .then(() => { + return this.routerService.transitionTo('parent.child', queryParams); + }) + .then(() => { + assert.ok(this.routerService.isActive('parent.child', this.buildQueryParams({ sort: 'descending' }))); + }); + } + + // ['@test RouterService#urlFor returns URL for simple route with null query params'](assert) { + // assert.expect(1); + + // let queryParams = buildQueryParams({ foo: null }); + + // return this.visit('/').then(() => { + // let expectedURL = this.routerService.urlFor('parent.child', queryParams); + + // assert.equal('/child', expectedURL); + // }); + // } + + // ['@test RouterService#urlFor returns URL for simple route with undefined query params'](assert) { + // assert.expect(1); + + // let queryParams = buildQueryParams({ foo: undefined }); + + // return this.visit('/').then(() => { + // let expectedURL = this.routerService.urlFor('parent.child', queryParams); + + // assert.equal('/child', expectedURL); + // }); + // } + + // ['@test RouterService#urlFor returns URL for simple route with dynamic segments and basic query params'](assert) { + // assert.expect(1); + + // let queryParams = buildQueryParams({ foo: 'bar' }); + + // return this.visit('/').then(() => { + // let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); + + // assert.equal('/dynamic/1?foo=bar', expectedURL); + // }); + // } + + // ['@test RouterService#urlFor returns URL for simple route with dynamic segments and array as query params'](assert) { + // assert.expect(1); + + // let queryParams = buildQueryParams({ selectedItems: ['a', 'b', 'c'] }); + + // return this.visit('/').then(() => { + // let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); + + // assert.equal('/dynamic/1?selectedItems[]=a&selectedItems[]=b&selectedItems[]=c', expectedURL); + // }); + // } + + // ['@test RouterService#urlFor returns URL for simple route with dynamic segments and null query params'](assert) { + // assert.expect(1); + + // let queryParams = buildQueryParams({ foo: null }); + + // return this.visit('/').then(() => { + // let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); + + // assert.equal('/dynamic/1', expectedURL); + // }); + // } + + // ['@test RouterService#urlFor returns URL for simple route with dynamic segments and undefined query params'](assert) { + // assert.expect(1); + + // let queryParams = buildQueryParams({ foo: undefined }); + + // return this.visit('/').then(() => { + // let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); + + // assert.equal('/dynamic/1', expectedURL); + // }); + // } + + // ['@test RouterService#urlFor correctly transitions to route via generated path'](assert) { + // assert.expect(1); + + // let expectedURL; + + // return this.visit('/') + // .then(() => { + // expectedURL = this.routerService.urlFor('parent.child'); + + // return this.routerService.transitionTo(expectedURL); + // }) + // .then(() => { + // assert.equal(expectedURL, this.routerService.get('currentURL')); + // }); + // } + + // ['@test RouterService#urlFor correctly transitions to route via generated path with dynamic segments'](assert) { + // assert.expect(1); + + // let expectedURL; + // let dynamicModel = { id: 1 }; + + // this.registerRoute('dynamic', Route.extend({ + // model() { + // return dynamicModel; + // } + // })); + + // return this.visit('/') + // .then(() => { + // expectedURL = this.routerService.urlFor('dynamic', dynamicModel); + + // return this.routerService.transitionTo(expectedURL); + // }) + // .then(() => { + // assert.equal(expectedURL, this.routerService.get('currentURL')); + // }); + // } + + // ['@test RouterService#urlFor correctly transitions to route via generated path with query params'](assert) { + // assert.expect(1); + + // let expectedURL; + // let actualURL; + // let queryParams = buildQueryParams({ foo: 'bar' }); + + // return this.visit('/') + // .then(() => { + // expectedURL = this.routerService.urlFor('parent.child', queryParams); + + // return this.routerService.transitionTo(expectedURL); + // }) + // .then(() => { + // actualURL = `${this.routerService.get('currentURL')}?foo=bar`; + + // assert.equal(expectedURL, actualURL); + // }); + // } + + // ['@test RouterService#urlFor correctly transitions to route via generated path with dynamic segments and query params'](assert) { + // assert.expect(1); + + // let expectedURL; + // let actualURL; + // let queryParams = buildQueryParams({ foo: 'bar' }); + // let dynamicModel = { id: 1 }; + + // this.registerRoute('dynamic', Route.extend({ + // model() { + // return dynamicModel; + // } + // })); + + // return this.visit('/') + // .then(() => { + // expectedURL = this.routerService.urlFor('dynamic', dynamicModel, queryParams); + + // return this.routerService.transitionTo(expectedURL); + // }) + // .then(() => { + // actualURL = `${this.routerService.get('currentURL')}?foo=bar`; + + // assert.equal(expectedURL, actualURL); + // }); + // } + }); +} diff --git a/packages/ember/tests/routing/router_service_test/urlFor_test.js b/packages/ember/tests/routing/router_service_test/urlFor_test.js index c17fcffa8e4..a0d8e74fd0b 100644 --- a/packages/ember/tests/routing/router_service_test/urlFor_test.js +++ b/packages/ember/tests/routing/router_service_test/urlFor_test.js @@ -3,7 +3,6 @@ import { inject, String } from 'ember-runtime'; -import { Component } from 'ember-glimmer'; import { Route, NoneLocation } from 'ember-routing'; import { get, @@ -26,12 +25,6 @@ function setupController(app, name) { }); } -function buildQueryParams(queryParams) { - return { - queryParams - }; -} - if (EMBER_ROUTING_ROUTER_SERVICE) { moduleFor('Router Service - urlFor', class extends RouterTestCase { ['@test RouterService#urlFor returns URL for simple route'](assert) { @@ -61,7 +54,7 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { ['@test RouterService#urlFor returns URL for simple route with basic query params'](assert) { assert.expect(1); - let queryParams = buildQueryParams({ foo: 'bar' }); + let queryParams = this.buildQueryParams({ foo: 'bar' }); return this.visit('/').then(() => { let expectedURL = this.routerService.urlFor('parent.child', queryParams); @@ -73,7 +66,7 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { ['@test RouterService#urlFor returns URL for simple route with array as query params'](assert) { assert.expect(1); - let queryParams = buildQueryParams({ selectedItems: ['a', 'b', 'c'] }); + let queryParams = this.buildQueryParams({ selectedItems: ['a', 'b', 'c'] }); return this.visit('/').then(() => { let expectedURL = this.routerService.urlFor('parent.child', queryParams); @@ -85,7 +78,7 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { ['@test RouterService#urlFor returns URL for simple route with null query params'](assert) { assert.expect(1); - let queryParams = buildQueryParams({ foo: null }); + let queryParams = this.buildQueryParams({ foo: null }); return this.visit('/').then(() => { let expectedURL = this.routerService.urlFor('parent.child', queryParams); @@ -97,7 +90,7 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { ['@test RouterService#urlFor returns URL for simple route with undefined query params'](assert) { assert.expect(1); - let queryParams = buildQueryParams({ foo: undefined }); + let queryParams = this.buildQueryParams({ foo: undefined }); return this.visit('/').then(() => { let expectedURL = this.routerService.urlFor('parent.child', queryParams); @@ -109,7 +102,7 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { ['@test RouterService#urlFor returns URL for simple route with dynamic segments and basic query params'](assert) { assert.expect(1); - let queryParams = buildQueryParams({ foo: 'bar' }); + let queryParams = this.buildQueryParams({ foo: 'bar' }); return this.visit('/').then(() => { let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); @@ -121,7 +114,7 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { ['@test RouterService#urlFor returns URL for simple route with dynamic segments and array as query params'](assert) { assert.expect(1); - let queryParams = buildQueryParams({ selectedItems: ['a', 'b', 'c'] }); + let queryParams = this.buildQueryParams({ selectedItems: ['a', 'b', 'c'] }); return this.visit('/').then(() => { let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); @@ -133,7 +126,7 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { ['@test RouterService#urlFor returns URL for simple route with dynamic segments and null query params'](assert) { assert.expect(1); - let queryParams = buildQueryParams({ foo: null }); + let queryParams = this.buildQueryParams({ foo: null }); return this.visit('/').then(() => { let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); @@ -145,7 +138,7 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { ['@test RouterService#urlFor returns URL for simple route with dynamic segments and undefined query params'](assert) { assert.expect(1); - let queryParams = buildQueryParams({ foo: undefined }); + let queryParams = this.buildQueryParams({ foo: undefined }); return this.visit('/').then(() => { let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); @@ -198,7 +191,7 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { let expectedURL; let actualURL; - let queryParams = buildQueryParams({ foo: 'bar' }); + let queryParams = this.buildQueryParams({ foo: 'bar' }); return this.visit('/') .then(() => { @@ -218,7 +211,7 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { let expectedURL; let actualURL; - let queryParams = buildQueryParams({ foo: 'bar' }); + let queryParams = this.buildQueryParams({ foo: 'bar' }); let dynamicModel = { id: 1 }; this.add('route:dynamic', Route.extend({ diff --git a/packages/internal-test-helpers/lib/test-cases/router.js b/packages/internal-test-helpers/lib/test-cases/router.js index 1beaa0519c3..4fc8488e4aa 100644 --- a/packages/internal-test-helpers/lib/test-cases/router.js +++ b/packages/internal-test-helpers/lib/test-cases/router.js @@ -17,4 +17,10 @@ export default class RouterTestCase extends ApplicationTestCase { get routerService() { return this.applicationInstance.lookup('service:router'); } + + buildQueryParams(queryParams) { + return { + queryParams + }; + } } From 3b9489cf6acb61c5ee22f8016f7486ef1e7133d4 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Sun, 25 Jun 2017 20:32:37 -0700 Subject: [PATCH 2/2] [FEATURE ember-routing-router-service] Cleanup and confirm expected behaviors. * Ensure that `routerService.transitionTo` and `routerService.replaceWith` does not elide query params (even the are the default values). * Update implementation of `routerService.isActive` to handle query params. * Ensure that the `router:main` injection property into the routerService is at `_router`. * Remove `routerService.currentState` * Add tests around various query param edge cases with `transitionTo` / `replaceWith` --- .../lib/system/application.js | 2 +- packages/ember-routing/lib/services/router.js | 77 ++++--- packages/ember-routing/lib/system/route.js | 2 +- packages/ember-routing/lib/system/router.js | 35 ++- .../ember-routing/lib/system/router_state.js | 1 - .../router_service_test/isActive_test.js | 200 +++--------------- .../router_service_test/replaceWith_test.js | 26 +++ .../router_service_test/transitionTo_test.js | 58 +++++ .../router_service_test/urlFor_test.js | 38 ++++ 9 files changed, 229 insertions(+), 210 deletions(-) diff --git a/packages/ember-application/lib/system/application.js b/packages/ember-application/lib/system/application.js index a9cfa3d870d..8c9e8876806 100644 --- a/packages/ember-application/lib/system/application.js +++ b/packages/ember-application/lib/system/application.js @@ -1045,7 +1045,7 @@ function commonSetupRegistry(registry) { if (EMBER_ROUTING_ROUTER_SERVICE) { registry.register('service:router', RouterService); - registry.injection('service:router', 'router', 'router:main'); + registry.injection('service:router', '_router', 'router:main'); } } diff --git a/packages/ember-routing/lib/services/router.js b/packages/ember-routing/lib/services/router.js index e9894c99631..eccdfaf0308 100644 --- a/packages/ember-routing/lib/services/router.js +++ b/packages/ember-routing/lib/services/router.js @@ -7,10 +7,6 @@ import { Service, readOnly } from 'ember-runtime'; -import { - get, - isEmpty -} from 'ember-metal'; import { assign } from 'ember-utils'; import RouterDSL from '../system/dsl'; @@ -35,12 +31,11 @@ function shallowEqual(a, b) { @category ember-routing-router-service */ const RouterService = Service.extend({ - currentRouteName: readOnly('router.currentRouteName'), - currentURL: readOnly('router.currentURL'), - currentState: readOnly('router.currentState'), - location: readOnly('router.location'), - rootURL: readOnly('router.rootURL'), - router: null, + currentRouteName: readOnly('_router.currentRouteName'), + currentURL: readOnly('_router.currentURL'), + location: readOnly('_router.location'), + rootURL: readOnly('_router.rootURL'), + _router: null, /** Transition the application into another route. The route may @@ -59,8 +54,25 @@ const RouterService = Service.extend({ attempted transition @public */ - transitionTo(/* routeNameOrUrl, ...models, options */) { - return this.router.transitionTo(...arguments); + transitionTo(...args) { + let queryParams; + let arg = args[0]; + if (resemblesURL(arg)) { + return this._router._doURLTransition('transitionTo', arg); + } + + let possibleQueryParams = args[args.length - 1]; + if (possibleQueryParams && possibleQueryParams.hasOwnProperty('queryParams')) { + queryParams = args.pop().queryParams; + } else { + queryParams = {}; + } + + let targetRouteName = args.shift(); + let transition = this._router._doTransition(targetRouteName, args, queryParams, true); + transition._keepDefaultQueryParamValues = true; + + return transition; }, /** @@ -81,13 +93,14 @@ const RouterService = Service.extend({ @public */ replaceWith(/* routeNameOrUrl, ...models, options */) { - return this.router.replaceWith(...arguments); + return this.transitionTo(...arguments).method('replace'); }, /** Generate a URL based on the supplied route name. @method urlFor + @category ember-routing-router-service @param {String} routeName the name of the route @param {...Object} models the model(s) or identifier(s) to be used while transitioning to the route. @@ -97,53 +110,53 @@ const RouterService = Service.extend({ @public */ urlFor(/* routeName, ...models, options */) { - return this.router.generate(...arguments); + return this._router.generate(...arguments); }, /** Determines whether a route is active. - @method urlFor + @method isActive + @category ember-routing-router-service @param {String} routeName the name of the route @param {...Object} models the model(s) or identifier(s) to be used while transitioning to the route. @param {Object} [options] optional hash with a queryParams property containing a mapping of query parameters - @return {String} the string representing the generated URL + @return {boolean} true if the provided routeName/models/queryParams are active @public */ isActive(/* routeName, ...models, options */) { - if (!this.router.isActive(...arguments)) { return false; } -debugger; let { routeName, models, queryParams } = this._extractArguments(...arguments); - let emptyQueryParams = Object.keys(queryParams).length; + let routerMicrolib = this._router._routerMicrolib; + let state = routerMicrolib.state; - if (!emptyQueryParams) { - let visibleQueryParams = {}; - assign(visibleQueryParams, queryParams); + if (!routerMicrolib.isActiveIntent(routeName, models, null)) { return false; } + let hasQueryParams = Object.keys(queryParams).length > 0; - this.router._prepareQueryParams(routeName, models, visibleQueryParams); - return shallowEqual(visibleQueryParams, queryParams); + if (hasQueryParams) { + this._router._prepareQueryParams(routeName, models, queryParams, true /* fromRouterService */); + return shallowEqual(queryParams, state.queryParams); } return true; }, - _extractArguments(...args) { - let routeName; - let models; - let possibleQueryParams = args[args.length - 1]; + _extractArguments(routeName, ...models) { + let possibleQueryParams = models[models.length - 1]; let queryParams = {}; if (possibleQueryParams && possibleQueryParams.hasOwnProperty('queryParams')) { - queryParams = args.pop().queryParams; + let options = models.pop(); + queryParams = options.queryParams; } - routeName = args.shift(); - models = args; - return { routeName, models, queryParams }; } }); +function resemblesURL(str) { + return typeof str === 'string' && (str === '' || str[0] === '/'); +} + export default RouterService; diff --git a/packages/ember-routing/lib/system/route.js b/packages/ember-routing/lib/system/route.js index 79c11d6c65e..07d92211c8e 100644 --- a/packages/ember-routing/lib/system/route.js +++ b/packages/ember-routing/lib/system/route.js @@ -873,7 +873,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { qp.serializedValue = svalue; let thisQueryParamHasDefaultValue = (qp.serializedDefaultValue === svalue); - if (!thisQueryParamHasDefaultValue) { + if (!thisQueryParamHasDefaultValue || transition._keepDefaultQueryParamValues) { finalParams.push({ value: svalue, visible: true, diff --git a/packages/ember-routing/lib/system/router.js b/packages/ember-routing/lib/system/router.js index f8f98f00fe9..6ffa5acefa9 100644 --- a/packages/ember-routing/lib/system/router.js +++ b/packages/ember-routing/lib/system/router.js @@ -767,7 +767,7 @@ const EmberRouter = EmberObject.extend(Evented, { } }, - _doTransition(_targetRouteName, models, _queryParams) { + _doTransition(_targetRouteName, models, _queryParams, _keepDefaultQueryParamValues) { let targetRouteName = _targetRouteName || getActiveTargetName(this._routerMicrolib); assert(`The route ${targetRouteName} was not found`, targetRouteName && this._routerMicrolib.hasRoute(targetRouteName)); @@ -776,7 +776,7 @@ const EmberRouter = EmberObject.extend(Evented, { this._processActiveTransitionQueryParams(targetRouteName, models, queryParams, _queryParams); assign(queryParams, _queryParams); - this._prepareQueryParams(targetRouteName, models, queryParams); + this._prepareQueryParams(targetRouteName, models, queryParams, _keepDefaultQueryParamValues); let transitionArgs = routeArgs(targetRouteName, models, queryParams); let transition = this._routerMicrolib.transitionTo(...transitionArgs); @@ -817,13 +817,17 @@ const EmberRouter = EmberObject.extend(Evented, { @param {String} targetRouteName @param {Array} models @param {Object} queryParams + @param {boolean} keepDefaultQueryParamValues @return {Void} */ - _prepareQueryParams(targetRouteName, models, queryParams) { + _prepareQueryParams(targetRouteName, models, queryParams, _fromRouterService) { let state = calculatePostTransitionState(this, targetRouteName, models); - this._hydrateUnsuppliedQueryParams(state, queryParams); + this._hydrateUnsuppliedQueryParams(state, queryParams, _fromRouterService); this._serializeQueryParams(state.handlerInfos, queryParams); - this._pruneDefaultQueryParamValues(state.handlerInfos, queryParams); + + if (!_fromRouterService) { + this._pruneDefaultQueryParamValues(state.handlerInfos, queryParams); + } }, /** @@ -945,7 +949,7 @@ const EmberRouter = EmberObject.extend(Evented, { @param {Object} queryParams @return {Void} */ - _hydrateUnsuppliedQueryParams(state, queryParams) { + _hydrateUnsuppliedQueryParams(state, queryParams, _fromRouterService) { let handlerInfos = state.handlerInfos; let appCache = this._bucketCache; @@ -955,12 +959,27 @@ const EmberRouter = EmberObject.extend(Evented, { if (!qpMeta) { continue; } for (let j = 0, qpLen = qpMeta.qps.length; j < qpLen; ++j) { - let qp = qpMeta.qps[j]; + var qp = qpMeta.qps[j]; - let presentProp = qp.prop in queryParams && qp.prop || + var presentProp = qp.prop in queryParams && qp.prop || qp.scopedPropertyName in queryParams && qp.scopedPropertyName || qp.urlKey in queryParams && qp.urlKey; + assert( + `You passed the \`${presentProp}\` query parameter during a transition into ${qp.route.routeName}, please update to ${qp.urlKey}`, + (function() { + if (qp.urlKey === presentProp) { + return true; + } + + if (_fromRouterService) { + return false; + } + + return true; + })() + ); + if (presentProp) { if (presentProp !== qp.scopedPropertyName) { queryParams[qp.scopedPropertyName] = queryParams[presentProp]; diff --git a/packages/ember-routing/lib/system/router_state.js b/packages/ember-routing/lib/system/router_state.js index 09a572348d2..446136de6b2 100644 --- a/packages/ember-routing/lib/system/router_state.js +++ b/packages/ember-routing/lib/system/router_state.js @@ -8,7 +8,6 @@ export default EmberObject.extend({ routerJsState: null, isActiveIntent(routeName, models, queryParams, queryParamsMustMatch) { - debugger; let state = this.routerJsState; if (!this.routerJs.isActiveIntent(routeName, models, null, state)) { return false; } diff --git a/packages/ember/tests/routing/router_service_test/isActive_test.js b/packages/ember/tests/routing/router_service_test/isActive_test.js index aaa91f9cf0a..1aad6807dc7 100644 --- a/packages/ember/tests/routing/router_service_test/isActive_test.js +++ b/packages/ember/tests/routing/router_service_test/isActive_test.js @@ -47,17 +47,41 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { }); } - ['@test RouterService#urlFor returns URL for simple route with basic query params'](assert) { - assert.expect(2); + ['@test RouterService#isActive does not eagerly instantiate controller for query params'](assert) { + assert.expect(1); let queryParams = this.buildQueryParams({ sort: 'ASC' }); - this.registerController('parent.child', Controller.extend({ - queryParams: ['sort'], - sort: 'ASC' + this.add('controller:parent.sister', Controller.extend({ + queryParams: ['sort'], + sort: 'ASC', + + init() { + assert.ok(false, 'should never create'); + this._super(...arguments); + } + })); + + return this.visit('/') + .then(() => { + return this.routerService.transitionTo('parent.brother'); }) - ); - debugger; + .then(() => { + assert.notOk(this.routerService.isActive('parent.sister', queryParams)); + }); + } + + ['@test RouterService#isActive is correct for simple route with basic query params'](assert) { + assert.expect(2); + + let queryParams = this.buildQueryParams({ sort: 'ASC' }); + + this.add('controller:parent.child', Controller.extend({ + queryParams: ['sort'], + sort: 'ASC' + }) + ); + return this.visit('/') .then(() => { return this.routerService.transitionTo('parent.child', queryParams); @@ -68,7 +92,7 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { }); } - ['@test RouterService#urlFor returns URL for simple route with array as query params'](assert) { + ['@test RouterService#isActive for simple route with array as query params'](assert) { assert.expect(1); let queryParams = this.buildQueryParams({ sort: ['ascending'] }); @@ -78,166 +102,8 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { return this.routerService.transitionTo('parent.child', queryParams); }) .then(() => { - assert.ok(this.routerService.isActive('parent.child', this.buildQueryParams({ sort: 'descending' }))); + assert.notOk(this.routerService.isActive('parent.child', this.buildQueryParams({ sort: 'descending' }))); }); } - - // ['@test RouterService#urlFor returns URL for simple route with null query params'](assert) { - // assert.expect(1); - - // let queryParams = buildQueryParams({ foo: null }); - - // return this.visit('/').then(() => { - // let expectedURL = this.routerService.urlFor('parent.child', queryParams); - - // assert.equal('/child', expectedURL); - // }); - // } - - // ['@test RouterService#urlFor returns URL for simple route with undefined query params'](assert) { - // assert.expect(1); - - // let queryParams = buildQueryParams({ foo: undefined }); - - // return this.visit('/').then(() => { - // let expectedURL = this.routerService.urlFor('parent.child', queryParams); - - // assert.equal('/child', expectedURL); - // }); - // } - - // ['@test RouterService#urlFor returns URL for simple route with dynamic segments and basic query params'](assert) { - // assert.expect(1); - - // let queryParams = buildQueryParams({ foo: 'bar' }); - - // return this.visit('/').then(() => { - // let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); - - // assert.equal('/dynamic/1?foo=bar', expectedURL); - // }); - // } - - // ['@test RouterService#urlFor returns URL for simple route with dynamic segments and array as query params'](assert) { - // assert.expect(1); - - // let queryParams = buildQueryParams({ selectedItems: ['a', 'b', 'c'] }); - - // return this.visit('/').then(() => { - // let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); - - // assert.equal('/dynamic/1?selectedItems[]=a&selectedItems[]=b&selectedItems[]=c', expectedURL); - // }); - // } - - // ['@test RouterService#urlFor returns URL for simple route with dynamic segments and null query params'](assert) { - // assert.expect(1); - - // let queryParams = buildQueryParams({ foo: null }); - - // return this.visit('/').then(() => { - // let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); - - // assert.equal('/dynamic/1', expectedURL); - // }); - // } - - // ['@test RouterService#urlFor returns URL for simple route with dynamic segments and undefined query params'](assert) { - // assert.expect(1); - - // let queryParams = buildQueryParams({ foo: undefined }); - - // return this.visit('/').then(() => { - // let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); - - // assert.equal('/dynamic/1', expectedURL); - // }); - // } - - // ['@test RouterService#urlFor correctly transitions to route via generated path'](assert) { - // assert.expect(1); - - // let expectedURL; - - // return this.visit('/') - // .then(() => { - // expectedURL = this.routerService.urlFor('parent.child'); - - // return this.routerService.transitionTo(expectedURL); - // }) - // .then(() => { - // assert.equal(expectedURL, this.routerService.get('currentURL')); - // }); - // } - - // ['@test RouterService#urlFor correctly transitions to route via generated path with dynamic segments'](assert) { - // assert.expect(1); - - // let expectedURL; - // let dynamicModel = { id: 1 }; - - // this.registerRoute('dynamic', Route.extend({ - // model() { - // return dynamicModel; - // } - // })); - - // return this.visit('/') - // .then(() => { - // expectedURL = this.routerService.urlFor('dynamic', dynamicModel); - - // return this.routerService.transitionTo(expectedURL); - // }) - // .then(() => { - // assert.equal(expectedURL, this.routerService.get('currentURL')); - // }); - // } - - // ['@test RouterService#urlFor correctly transitions to route via generated path with query params'](assert) { - // assert.expect(1); - - // let expectedURL; - // let actualURL; - // let queryParams = buildQueryParams({ foo: 'bar' }); - - // return this.visit('/') - // .then(() => { - // expectedURL = this.routerService.urlFor('parent.child', queryParams); - - // return this.routerService.transitionTo(expectedURL); - // }) - // .then(() => { - // actualURL = `${this.routerService.get('currentURL')}?foo=bar`; - - // assert.equal(expectedURL, actualURL); - // }); - // } - - // ['@test RouterService#urlFor correctly transitions to route via generated path with dynamic segments and query params'](assert) { - // assert.expect(1); - - // let expectedURL; - // let actualURL; - // let queryParams = buildQueryParams({ foo: 'bar' }); - // let dynamicModel = { id: 1 }; - - // this.registerRoute('dynamic', Route.extend({ - // model() { - // return dynamicModel; - // } - // })); - - // return this.visit('/') - // .then(() => { - // expectedURL = this.routerService.urlFor('dynamic', dynamicModel, queryParams); - - // return this.routerService.transitionTo(expectedURL); - // }) - // .then(() => { - // actualURL = `${this.routerService.get('currentURL')}?foo=bar`; - - // assert.equal(expectedURL, actualURL); - // }); - // } }); } diff --git a/packages/ember/tests/routing/router_service_test/replaceWith_test.js b/packages/ember/tests/routing/router_service_test/replaceWith_test.js index eae51024eb0..40770d281ba 100644 --- a/packages/ember/tests/routing/router_service_test/replaceWith_test.js +++ b/packages/ember/tests/routing/router_service_test/replaceWith_test.js @@ -4,6 +4,7 @@ import { moduleFor } from 'internal-test-helpers'; import { Transition } from 'router'; +import { Controller } from 'ember-runtime'; import { EMBER_ROUTING_ROUTER_SERVICE } from 'ember/features'; @@ -105,5 +106,30 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { assert.deepEqual(this.state, ['/', '/child', '/sister', '/sister']); }); } + + ['@test RouterService#replaceWith with basic query params does not remove query param defaults'](assert) { + assert.expect(1); + + this.add('controller:parent.child', Controller.extend({ + queryParams: ['sort'], + sort: 'ASC' + })); + + let queryParams = this.buildQueryParams({ sort: 'ASC' }); + + return this.visit('/') + .then(() => { + return this.routerService.transitionTo('parent.brother'); + }) + .then(() => { + return this.routerService.replaceWith('parent.sister'); + }) + .then(() => { + return this.routerService.replaceWith('parent.child', queryParams); + }) + .then(() => { + assert.deepEqual(this.state, ['/', '/child?sort=ASC']); + }); + } }); } diff --git a/packages/ember/tests/routing/router_service_test/transitionTo_test.js b/packages/ember/tests/routing/router_service_test/transitionTo_test.js index cdd836c6564..ab98ce08570 100644 --- a/packages/ember/tests/routing/router_service_test/transitionTo_test.js +++ b/packages/ember/tests/routing/router_service_test/transitionTo_test.js @@ -1,6 +1,7 @@ import { inject } from 'ember-runtime'; import { Component } from 'ember-glimmer'; import { Route, NoneLocation } from 'ember-routing'; +import { Controller } from 'ember-runtime'; import { run, get, @@ -236,5 +237,62 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { this.assertText('much dynamicism'); }); } + + ['@test RouterService#transitionTo with basic query params does not remove query param defaults'](assert) { + assert.expect(1); + + this.add('controller:parent.child', Controller.extend({ + queryParams: ['sort'], + sort: 'ASC' + })); + + let queryParams = this.buildQueryParams({ sort: 'ASC' }); + + return this.visit('/').then(() => { + return this.routerService.transitionTo('parent.child', queryParams); + }) + .then(() => { + assert.equal(this.routerService.get('currentURL'), '/child?sort=ASC'); + }); + } + + ['@test RouterService#transitionTo with aliased query params uses the original provided key'](assert) { + assert.expect(1); + + this.add('controller:parent.child', Controller.extend({ + queryParams: { + 'cont_sort': 'url_sort' + }, + cont_sort: 'ASC' + })); + + let queryParams = this.buildQueryParams({ url_sort: 'ASC' }); + + return this.visit('/').then(() => { + return this.routerService.transitionTo('parent.child', queryParams); + }) + .then(() => { + assert.equal(this.routerService.get('currentURL'), '/child?url_sort=ASC'); + }); + } + + ['@test RouterService#transitionTo with aliased query params uses the original provided key when controller property name'](assert) { + assert.expect(1); + + this.add('controller:parent.child', Controller.extend({ + queryParams: { + 'cont_sort': 'url_sort' + }, + cont_sort: 'ASC' + })); + + let queryParams = this.buildQueryParams({ cont_sort: 'ASC' }); + + return this.visit('/').then(() => { + expectAssertion(() => { + return this.routerService.transitionTo('parent.child', queryParams); + }, 'You passed the `cont_sort` query parameter during a transition into parent.child, please update to url_sort'); + }); + } }); } diff --git a/packages/ember/tests/routing/router_service_test/urlFor_test.js b/packages/ember/tests/routing/router_service_test/urlFor_test.js index a0d8e74fd0b..92ae2fa3178 100644 --- a/packages/ember/tests/routing/router_service_test/urlFor_test.js +++ b/packages/ember/tests/routing/router_service_test/urlFor_test.js @@ -63,6 +63,44 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { }); } + ['@test RouterService#urlFor returns URL for simple route with basic query params and default value'](assert) { + assert.expect(1); + + this.add('controller:parent.child', Controller.extend({ + queryParams: ['sort'], + sort: 'ASC' + })); + + let queryParams = this.buildQueryParams({ sort: 'ASC' }); + + return this.visit('/').then(() => { + let expectedURL = this.routerService.urlFor('parent.child', queryParams); + + assert.equal('/child?sort=ASC', expectedURL); + }); + } + + ['@test RouterService#urlFor returns URL for simple route with basic query params and default value with stickyness'](assert) { + assert.expect(2); + + this.add('controller:parent.child', Controller.extend({ + queryParams: ['sort', 'foo'], + sort: 'ASC' + })); + + let queryParams = this.buildQueryParams({ sort: 'DESC' }); + + return this.visit('/child/?sort=DESC').then(() => { + let controller = this.applicationInstance.lookup('controller:parent.child'); + assert.equal(get(controller, 'sort'), 'DESC', 'sticky is set'); + + let queryParams = this.buildQueryParams({ foo: 'derp' }); + let actual = this.routerService.urlFor('parent.child', queryParams); + + assert.equal(actual, '/child?foo=derp', 'does not use "stickiness"'); + }); + } + ['@test RouterService#urlFor returns URL for simple route with array as query params'](assert) { assert.expect(1);