Skip to content
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.

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

[REFACTOR] Lazily reify router’s location #10483

Merged
merged 3 commits into from
Mar 5, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
46 changes: 31 additions & 15 deletions packages/ember-application/lib/system/application-instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
@private
*/

import { get } from "ember-metal/property_get";
import { set } from "ember-metal/property_set";
import EmberObject from "ember-runtime/system/object";
import run from "ember-metal/run_loop";
import { computed } from "ember-metal/computed";
import Registry from 'container/registry';

/**
Expand Down Expand Up @@ -103,23 +105,24 @@ export default EmberObject.extend({
this.registry.register('-application-instance:main', this, { instantiate: false });
},

router: computed(function() {
return this.container.lookup('router:main');
}).readOnly(),

/**
Instantiates and sets up the router, optionally overriding the default
Instantiates and sets up the router, specifically overriding the default
location. This is useful for manually starting the app in FastBoot or
testing environments, where trying to modify the URL would be
inappropriate.

@param options
@private
*/
setupRouter: function(options) {
var router = this.container.lookup('router:main');
overrideRouterLocation: function(options) {
var location = options && options.location;
var router = get(this, 'router');

var location = options.location;
if (location) { set(router, 'location', location); }

router._setupLocation();
router.setupRouter(true);
},

/**
Expand All @@ -143,17 +146,30 @@ export default EmberObject.extend({
current URL of the page to determine the initial URL to start routing to.
To start the app at a specific URL, call `handleURL` instead.

Ensure that you have called `setupRouter()` on the instance before using
this method.

@private
*/
startRouting: function() {
var router = this.container.lookup('router:main');
if (!router) { return; }

var router = get(this, 'router');
var isModuleBasedResolver = !!this.registry.resolver.moduleBasedResolver;

router.startRouting(isModuleBasedResolver);
this._didSetupRouter = true;
},

/** @private
Sets up the router, initializing the child router and configuring the
location before routing begins.

Because setup should only occur once, multiple calls to `setupRouter`
beyond the first call have no effect.
*/
setupRouter: function() {
if (this._didSetupRouter) { return; }
this._didSetupRouter = true;

var router = get(this, 'router');
var isModuleBasedResolver = !!this.registry.resolver.moduleBasedResolver;
router.setupRouter(isModuleBasedResolver);
},

/**
Expand All @@ -165,8 +181,9 @@ export default EmberObject.extend({
@private
*/
handleURL: function(url) {
var router = this.container.lookup('router:main');
var router = get(this, 'router');

this.setupRouter();
return router.handleURL(url);
},

Expand All @@ -175,7 +192,6 @@ export default EmberObject.extend({
*/
setupEventDispatcher: function() {
var dispatcher = this.container.lookup('event_dispatcher:main');

dispatcher.setup(this.customEvents, this.rootElement);

return dispatcher;
Expand Down
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 @@ -817,7 +817,7 @@ if (Ember.FEATURES.isEnabled('ember-application-visit')) {
};
});

instance.setupRouter({ location: 'none' });
instance.overrideRouterLocation({ location: 'none' });

return instance.handleURL(url).then(function() {
return renderPromise;
Expand Down
5 changes: 2 additions & 3 deletions packages/ember-routing/lib/system/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ var EmberRouter = EmberObject.extend(Evented, {

init: function() {
this._activeViews = {};
this._setupLocation();
this._qpCache = {};
this._queuedQPChanges = {};
},
Expand All @@ -126,9 +125,8 @@ var EmberRouter = EmberObject.extend(Evented, {
*/
startRouting: function(moduleBasedResolver) {
var initialURL = get(this, 'initialURL');
var location = get(this, 'location');

if (this.setupRouter(moduleBasedResolver, location)) {
if (this.setupRouter(moduleBasedResolver)) {
if (typeof initialURL === "undefined") {
initialURL = get(this, 'location').getURL();
}
Expand All @@ -141,6 +139,7 @@ var EmberRouter = EmberObject.extend(Evented, {

setupRouter: function(moduleBasedResolver) {
this._initRouterJs(moduleBasedResolver);
this._setupLocation();

var router = this.router;
var location = get(this, 'location');
Expand Down
22 changes: 18 additions & 4 deletions packages/ember-routing/tests/system/router_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,16 @@ import { runDestroy } from "ember-runtime/tests/utils";

var registry, container;

function createRouter(overrides) {
function createRouter(overrides, disableSetup) {
var opts = merge({ container: container }, overrides);
var routerWithContainer = Router.extend();
var router = routerWithContainer.create(opts);

return routerWithContainer.create(opts);
if (!disableSetup) {
router.setupRouter();
}

return router;
}

QUnit.module("Ember Router", {
Expand All @@ -35,17 +40,26 @@ QUnit.module("Ember Router", {
});

QUnit.test("can create a router without a container", function() {
createRouter({ container: null });
createRouter({ container: null }, true);

ok(true, 'no errors were thrown when creating without a container');
});

QUnit.test("should not create a router.js instance upon init", function() {
var router = createRouter();
var router = createRouter(null, true);

ok(!router.router);
});

QUnit.test("should not reify location until setupRouter is called", function() {
var router = createRouter(null, true);
equal(typeof router.location, 'string', "location is specified as a string");

router.setupRouter();

equal(typeof router.location, 'object', "location is reified into an object");
});

QUnit.test("should destroy its location upon destroying the routers container.", function() {
var router = createRouter();
var location = router.get('location');
Expand Down
2 changes: 1 addition & 1 deletion packages/ember-testing/lib/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@ function focus(el) {

function visit(app, url) {
var router = app.__container__.lookup('router:main');
router.location.setURL(url);

if (app._readinessDeferrals > 0) {
router['initialURL'] = url;
run(app, 'advanceReadiness');
delete router['initialURL'];
} else {
router.location.setURL(url);
run(app.__deprecatedInstance__, 'handleURL', url);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/ember-testing/tests/helpers_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ QUnit.test("Ember.Application#setupForTesting", function() {
App.setupForTesting();
});

equal(App.__container__.lookup('router:main').location.implementation, 'none');
equal(App.__container__.lookup('router:main').location, 'none');
});

QUnit.test("Ember.Application.setupForTesting sets the application to `testing`.", function() {
Expand Down