Skip to content

Commit

Permalink
[FEATURE ember-routing-router-service] Cleanup and confirm expected b…
Browse files Browse the repository at this point in the history
…ehaviors.

* 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`
  • Loading branch information
rwjblue committed Jun 26, 2017
1 parent 5314c87 commit 91b5abe
Show file tree
Hide file tree
Showing 9 changed files with 228 additions and 209 deletions.
2 changes: 1 addition & 1 deletion packages/ember-application/lib/system/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}

Expand Down
75 changes: 44 additions & 31 deletions packages/ember-routing/lib/services/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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
Expand All @@ -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;
},

/**
Expand All @@ -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.
Expand All @@ -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;
_extractArguments(routeName, ...models) {
let possibleQueryParams = args[args.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;
2 changes: 1 addition & 1 deletion packages/ember-routing/lib/system/route.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
35 changes: 27 additions & 8 deletions packages/ember-routing/lib/system/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand All @@ -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);
Expand Down Expand Up @@ -817,13 +817,17 @@ const EmberRouter = EmberObject.extend(Evented, {
@param {String} targetRouteName
@param {Array<Object>} 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);
}
},

/**
Expand Down Expand Up @@ -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;

Expand All @@ -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];
Expand Down
1 change: 0 additions & 1 deletion packages/ember-routing/lib/system/router_state.js
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down
Loading

0 comments on commit 91b5abe

Please sign in to comment.