Skip to content

Introduce routing service #10596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Introduce routing service #10596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 33 additions & 6 deletions packages/ember-routing-htmlbars/tests/helpers/link-to_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,32 @@ import EmberView from "ember-views/views/view";
import compile from "ember-template-compiler/system/compile";
import { set } from "ember-metal/property_set";
import Controller from "ember-runtime/controllers/controller";
import { Registry } from "ember-runtime/system/container";
import { runAppend, runDestroy } from "ember-runtime/tests/utils";
import EmberObject from "ember-runtime/system/object";

var view;
var container;
var registry = new Registry();

// These tests don't rely on the routing service, but LinkView makes
// some assumptions that it will exist. This small stub service ensures
// that the LinkView can render without raising an exception.
//
// TODO: Add tests that test actual behavior. Currently, all behavior
// is tested integration-style in the `ember` package.
registry.register('service:-routing', EmberObject.extend({
availableRoutes: function() { return ['index']; },
hasRoute: function(name) { return name === 'index'; },
isActiveForRoute: function() { return true; },
generateURL: function() { return "/"; }
}));

QUnit.module("ember-routing-htmlbars: link-to helper", {
setup: function() {
container = registry.container();
},

teardown: function() {
runDestroy(view);
}
Expand All @@ -18,7 +39,8 @@ QUnit.module("ember-routing-htmlbars: link-to helper", {
QUnit.test("should be able to be inserted in DOM when the router is not present", function() {
var template = "{{#link-to 'index'}}Go to Index{{/link-to}}";
view = EmberView.create({
template: compile(template)
template: compile(template),
container: container
});

runAppend(view);
Expand All @@ -33,7 +55,8 @@ QUnit.test("re-renders when title changes", function() {
title: 'foo',
routeName: 'index'
},
template: compile(template)
template: compile(template),
container: container
});

runAppend(view);
Expand All @@ -54,7 +77,8 @@ QUnit.test("can read bound title", function() {
title: 'foo',
routeName: 'index'
},
template: compile(template)
template: compile(template),
container: container
});

runAppend(view);
Expand All @@ -65,7 +89,8 @@ QUnit.test("can read bound title", function() {
QUnit.test("escaped inline form (double curlies) escapes link title", function() {
view = EmberView.create({
title: "<b>blah</b>",
template: compile("{{link-to view.title}}")
template: compile("{{link-to view.title}}"),
container: container
});

runAppend(view);
Expand All @@ -76,7 +101,8 @@ QUnit.test("escaped inline form (double curlies) escapes link title", function()
QUnit.test("unescaped inline form (triple curlies) does not escape link title", function() {
view = EmberView.create({
title: "<b>blah</b>",
template: compile("{{{link-to view.title}}}")
template: compile("{{{link-to view.title}}}"),
container: container
});

runAppend(view);
Expand All @@ -92,7 +118,8 @@ QUnit.test("unwraps controllers", function() {
model: 'foo'
}),

template: compile(template)
template: compile(template),
container: container
});

expectDeprecation(function() {
Expand Down
166 changes: 31 additions & 135 deletions packages/ember-routing-views/lib/views/link.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,12 @@
import Ember from "ember-metal/core"; // FEATURES, Logger, assert

import { get } from "ember-metal/property_get";
import merge from "ember-metal/merge";
import run from "ember-metal/run_loop";
import { computed } from "ember-metal/computed";
import { fmt } from "ember-runtime/system/string";
import keys from "ember-metal/keys";
import { isSimpleClick } from "ember-views/system/utils";
import EmberComponent from "ember-views/views/component";
import { routeArgs } from "ember-routing/utils";
import { read, subscribe } from "ember-metal/streams/utils";

var numberOfContextsAcceptedByHandler = function(handler, handlerInfos) {
var req = 0;
for (var i = 0, l = handlerInfos.length; i < l; i++) {
req = req + handlerInfos[i].names.length;
if (handlerInfos[i].handler === handler) {
break;
}
}

return req;
};
import inject from "ember-runtime/inject";

var linkViewClassNameBindings = ['active', 'loading', 'disabled'];
if (Ember.FEATURES.isEnabled('ember-routing-transitioning-classes')) {
Expand Down Expand Up @@ -216,6 +201,8 @@ var LinkView = EmberComponent.extend({
this.on(eventName, this, this._invoke);
},

_routing: inject.service('-routing'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about we call it routingService

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stefanpenner What's the rationale?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think _routing lacks clarity

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice, I think of services as being like specialized controllers. Before services, people would have things like App.AuthenticationController, then use needs to access it where they needed it. In almost every example I've seen, it's aliased as [noun] rather than [noun]Controller.

App.UserController = Ember.Controller.extend({
  needs: ['authentication'],

  authentication: alias('controllers.authentication')
});

Additionally, in the most recent Ember Data, the store has been refactored to be a service. In order to be consistent with your proposal, we should change the injected property name to storeService rather than store, but this case doesn't seem to buy us much clarity.

I'd rather us be consistent with service naming across the spectrum, and I think, if considered holistically, not suffixing services with Service is consistent with how we typically name things.

(I'm open to argument here, would especially love to hear from others; just wanted to jot down my thought process.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my opinion on this above comment should be considered with a weight of 4/10


/**
This method is invoked by observers installed during `init` that fire
whenever the params change
Expand Down Expand Up @@ -290,16 +277,14 @@ var LinkView = EmberComponent.extend({
@property active
**/
active: computed('loadedParams', function computeLinkViewActive() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

readOnly

var router = get(this, 'router');
if (!router) { return; }
return computeActive(this, router.currentState);
var currentState = get(this, '_routing.currentState');
return computeActive(this, currentState);
}),

willBeActive: computed('router.targetState', function() {
var router = get(this, 'router');
if (!router) { return; }
var targetState = router.targetState;
if (router.currentState === targetState) { return; }
willBeActive: computed('_routing.targetState', function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

readOnly

var routing = get(this, '_routing');
var targetState = get(routing, 'targetState');
if (get(routing, 'currentState') === targetState) { return; }

return !!computeActive(this, targetState);
}),
Expand Down Expand Up @@ -333,19 +318,6 @@ var LinkView = EmberComponent.extend({
if (!get(this, 'loadedParams')) { return get(this, 'loadingClass'); }
}),

/**
Returns the application's main router from the container.

@private
@property router
**/
router: computed(function() {
var controller = get(this, 'controller');
if (controller && controller.container) {
return controller.container.lookup('router:main');
}
}),

/**
Event handler that invokes the link, activating the associated route.

Expand Down Expand Up @@ -377,57 +349,8 @@ var LinkView = EmberComponent.extend({
return false;
}

var router = get(this, 'router');
var loadedParams = get(this, 'loadedParams');

var transition = router._doTransition(loadedParams.targetRouteName, loadedParams.models, loadedParams.queryParams);
if (get(this, 'replace')) {
transition.method('replace');
}

if (Ember.FEATURES.isEnabled('ember-routing-transitioning-classes')) {
return;
}

// Schedule eager URL update, but after we've given the transition
// a chance to synchronously redirect.
// We need to always generate the URL instead of using the href because
// the href will include any rootURL set, but the router expects a URL
// without it! Note that we don't use the first level router because it
// calls location.formatURL(), which also would add the rootURL!
var args = routeArgs(loadedParams.targetRouteName, loadedParams.models, transition.state.queryParams);
var url = router.router.generate.apply(router.router, args);

run.scheduleOnce('routerTransitions', this, this._eagerUpdateUrl, transition, url);
},

/**
@private
@method _eagerUpdateUrl
@param transition
@param href
*/
_eagerUpdateUrl: function(transition, href) {
if (!transition.isActive || !transition.urlMethod) {
// transition was aborted, already ran to completion,
// or it has a null url-updated method.
return;
}

if (href.indexOf('#') === 0) {
href = href.slice(1);
}

// Re-use the routerjs hooks set up by the Ember router.
var routerjs = get(this, 'router.router');
if (transition.urlMethod === 'update') {
routerjs.updateURL(href);
} else if (transition.urlMethod === 'replace') {
routerjs.replaceURL(href);
}

// Prevent later update url refire.
transition.method(null);
var params = get(this, 'loadedParams');
get(this, '_routing').transitionTo(params.targetRouteName, params.models, params.queryParams, get(this, 'replace'));
},

/**
Expand All @@ -449,15 +372,14 @@ var LinkView = EmberComponent.extend({
@property
@return {Array}
*/
resolvedParams: computed('router.url', function() {
resolvedParams: computed('_routing.currentState', function() {
var params = this.params;
var targetRouteName;
var models = [];
var onlyQueryParamsSupplied = (params.length === 0);

if (onlyQueryParamsSupplied) {
var appController = this.container.lookup('controller:application');
targetRouteName = get(appController, 'currentRouteName');
targetRouteName = get(this, '_routing.currentRouteName');
} else {
targetRouteName = read(params[0]);

Expand Down Expand Up @@ -486,8 +408,8 @@ var LinkView = EmberComponent.extend({
@return {Array} An array with the route name and any dynamic segments
**/
loadedParams: computed('resolvedParams', function computeLinkViewRouteArgs() {
var router = get(this, 'router');
if (!router) { return; }
var routing = get(this, '_routing');
if (!routing) { return; }

var resolvedParams = get(this, 'resolvedParams');
var namedRoute = resolvedParams.targetRouteName;
Expand All @@ -496,8 +418,8 @@ var LinkView = EmberComponent.extend({

Ember.assert(fmt("The attempt to link-to route '%@' failed. " +
"The router did not find '%@' in its possible routes: '%@'",
[namedRoute, namedRoute, keys(router.router.recognizer.names).join("', '")]),
router.hasRoute(namedRoute));
[namedRoute, namedRoute, routing.availableRoutes().join("', '")]),
routing.hasRoute(namedRoute));

if (!paramsAreLoaded(resolvedParams.models)) { return; }

Expand All @@ -518,20 +440,14 @@ var LinkView = EmberComponent.extend({
href: computed('loadedParams', function computeLinkViewHref() {
if (get(this, 'tagName') !== 'a') { return; }

var router = get(this, 'router');
var loadedParams = get(this, 'loadedParams');
var routing = get(this, '_routing');
var params = get(this, 'loadedParams');

if (!loadedParams) {
if (!params) {
return get(this, 'loadingHref');
}

var visibleQueryParams = {};
merge(visibleQueryParams, loadedParams.queryParams);
router._prepareQueryParams(loadedParams.targetRouteName, loadedParams.models, visibleQueryParams);

var args = routeArgs(loadedParams.targetRouteName, loadedParams.models, visibleQueryParams);
var result = router.generate.apply(router, args);
return result;
return routing.generateURL(params.targetRouteName, params.models, params.queryParams);
}),

/**
Expand Down Expand Up @@ -572,46 +488,26 @@ function paramsAreLoaded(params) {
return true;
}

function computeActive(route, routerState) {
if (get(route, 'loading')) { return false; }
function computeActive(view, routerState) {
if (get(view, 'loading')) { return false; }

var currentWhen = route['current-when'] || route.currentWhen;
var currentWhen = view['current-when'] || view.currentWhen;
var isCurrentWhenSpecified = !!currentWhen;
currentWhen = currentWhen || get(route, 'loadedParams').targetRouteName;
currentWhen = currentWhen || get(view, 'loadedParams').targetRouteName;
currentWhen = currentWhen.split(' ');
for (var i = 0, len = currentWhen.length; i < len; i++) {
if (isActiveForRoute(route, currentWhen[i], isCurrentWhenSpecified, routerState)) {
return get(route, 'activeClass');
if (isActiveForRoute(view, currentWhen[i], isCurrentWhenSpecified, routerState)) {
return get(view, 'activeClass');
}
}

return false;
}

function isActiveForRoute(route, routeName, isCurrentWhenSpecified, routerState) {
var router = get(route, 'router');
var loadedParams = get(route, 'loadedParams');
var contexts = loadedParams.models;

var handlers = router.router.recognizer.handlersFor(routeName);
var leafName = handlers[handlers.length-1].handler;
var maximumContexts = numberOfContextsAcceptedByHandler(routeName, handlers);

// NOTE: any ugliness in the calculation of activeness is largely
// due to the fact that we support automatic normalizing of
// `resource` -> `resource.index`, even though there might be
// dynamic segments / query params defined on `resource.index`
// which complicates (and makes somewhat ambiguous) the calculation
// of activeness for links that link to `resource` instead of
// directly to `resource.index`.

// if we don't have enough contexts revert back to full route name
// this is because the leaf route will use one of the contexts
if (contexts.length > maximumContexts) {
routeName = leafName;
}

return routerState.isActiveIntent(routeName, contexts, loadedParams.queryParams, !isCurrentWhenSpecified);
function isActiveForRoute(view, routeName, isCurrentWhenSpecified, routerState) {
var params = get(view, 'loadedParams');
var service = get(view, '_routing');
return service.isActiveForRoute(params, routeName, routerState, isCurrentWhenSpecified);
}

export {
Expand Down
14 changes: 14 additions & 0 deletions packages/ember-routing/lib/initializers/routing-service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { onLoad } from "ember-runtime/system/lazy_load";
import RoutingService from "ember-routing/services/routing";

onLoad('Ember.Application', function(Application) {
Application.initializer({
name: 'routing-service',
initialize: function(registry) {
// Register the routing service...
registry.register('service:-routing', RoutingService);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not part of the default buildContainer stuff?

// Then inject the app router into it
registry.injection('service:-routing', 'router', 'router:main');
}
});
});
2 changes: 2 additions & 0 deletions packages/ember-routing/lib/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import RouterDSL from "ember-routing/system/dsl";
import Router from "ember-routing/system/router";
import Route from "ember-routing/system/route";

import "ember-routing/initializers/routing-service";

Ember.Location = EmberLocation;
Ember.AutoLocation = AutoLocation;
Ember.HashLocation = HashLocation;
Expand Down
Loading