Skip to content

Commit

Permalink
[BUGFIX canary] Decouple route transition from view creation
Browse files Browse the repository at this point in the history
This closes #9814 and closes #10304, which are examples of a class of
problems caused by the way the router synchronously reaches into the
view hierarchy to set outlet state. It is a significant refactor of the
way the router communicates state to outlets.

What motivates this change?

 - Simple examples like `{{#if something}}{{outlet}}{{/if}}` are
   incorrect under the current model.

 - Richer examples like block-helpers to enable animation also
   suffer. In general, the router cannot know when and whether a
   particular outlet is going to exist, and it shouldn't need to know.

 - The router maintains a bunch of view-related state that is actually
   redundant with the view hierarchy itself, leading to unnecessary
   complexity.

 - This eliminates the longstanding weirdness & confusion caused by the
   fact that we used to create new `View` instances and then throw them
   away if they looked similar enough to ones that were already
   rendered. That functionality is now covered by state diffing in the
   `OutletView`.

 - We reduce the API surface area between router and view layer in a way
   that should make it easier to experiment with swapping in compatible
   implementations of either.

 - As a bonus, this changes makes outlets work in an observer-free way
   that will make them easy to integrate with upcoming planned view
   layer optimizations.

How does this work?

 - Rather than directly building and linking views, the router builds up
   an abstract summary of the render decisions that have been made by
   the current routes.

 - This state is cheap to recalculate as needed. It doesn't do any view
   creation. To avoid expensive observer creation & teardown, we just
   recreate the whole thing and use a `setState`-like mechanism to
   propagate the changes through the outlet hierarchy. This gives us
   optimal granularity of updates.

 - Actual view instantiation moves into the OutletView -- within the
   view layer where it belongs. Each outlet does a diff to see whether
   it should rerender itself or propagate inner changes down to its
   child outlets.

 - To bootstrap rendering, the router creates a single top-level outlet,
   after which all view creation is internal to the view layer.

Does this break any existing semantics?

 - No, as far as I can tell.

Could this get even better if we decided to deprecate some old
semantics?

 - Yes. It would be better if users` `renderTemplate` implementations on
   `Route`s were required to be idempotent. Then we could eliminate a
   bunch of the remaining state from them.

 - Also, when we deprecate the `render` helper we can eliminate the
   remaining use of `_activeViews` state tracking on the router. That is
   the only remaining use for it.
  • Loading branch information
ef4 authored and rwjblue committed Feb 8, 2015
1 parent b44618c commit e047a91
Show file tree
Hide file tree
Showing 13 changed files with 525 additions and 522 deletions.
3 changes: 3 additions & 0 deletions packages/ember-application/lib/system/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import ArrayController from "ember-runtime/controllers/array_controller";
import Renderer from "ember-views/system/renderer";
import DOMHelper from "dom-helper";
import SelectView from "ember-views/views/select";
import { OutletView } from "ember-routing-views/views/outlet";
import EmberView from "ember-views/views/view";
import _MetamorphView from "ember-views/views/metamorph_view";
import EventDispatcher from "ember-views/system/event_dispatcher";
Expand Down Expand Up @@ -1003,6 +1004,7 @@ Application.reopenClass({

registry.injection('view', 'renderer', 'renderer:-dom');
registry.register('view:select', SelectView);
registry.register('view:-outlet', OutletView);

registry.register('view:default', _MetamorphView);
registry.register('view:toplevel', EmberView.extend());
Expand All @@ -1011,6 +1013,7 @@ Application.reopenClass({
registry.register('event_dispatcher:main', EventDispatcher);

registry.injection('router:main', 'namespace', 'application:main');
registry.injection('view:-outlet', 'namespace', 'application:main');

registry.register('location:auto', AutoLocation);
registry.register('location:hash', HashLocation);
Expand Down
5 changes: 3 additions & 2 deletions packages/ember-application/tests/system/logging_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Controller from "ember-runtime/controllers/controller";
import Route from "ember-routing/system/route";
import RSVP from "ember-runtime/ext/rsvp";
import keys from "ember-metal/keys";
import compile from "ember-template-compiler/system/compile";

import "ember-routing";

Expand Down Expand Up @@ -182,7 +183,7 @@ QUnit.test("log when template and view are missing when flag is active", functio
return;
}

App.register('template:application', function() { return ''; });
App.register('template:application', compile("{{outlet}}"));
run(App, 'advanceReadiness');

visit('/posts').then(function() {
Expand Down Expand Up @@ -210,7 +211,7 @@ QUnit.test("log which view is used with a template", function() {
return;
}

App.register('template:application', function() { return 'Template with default view'; });
App.register('template:application', compile('{{outlet}}'));
App.register('template:foo', function() { return 'Template with custom view'; });
App.register('view:posts', View.extend({ templateName: 'foo' }));
run(App, 'advanceReadiness');
Expand Down
15 changes: 2 additions & 13 deletions packages/ember-routing-htmlbars/lib/helpers/outlet.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
*/

import Ember from "ember-metal/core"; // assert
import { set } from "ember-metal/property_set";
import { OutletView } from "ember-routing-views/views/outlet";

/**
The `outlet` helper is a placeholder that the router will fill in with
Expand Down Expand Up @@ -71,7 +69,6 @@ import { OutletView } from "ember-routing-views/views/outlet";
@return {String} HTML string
*/
export function outletHelper(params, hash, options, env) {
var outletSource;
var viewName;
var viewClass;
var viewFullName;
Expand All @@ -83,11 +80,6 @@ export function outletHelper(params, hash, options, env) {

var property = params[0] || 'main';

outletSource = this;
while (!outletSource.get('template.isTop')) {
outletSource = outletSource._parentView;
}
set(this, 'outletSource', outletSource);

// provide controller override
viewName = hash.view;
Expand All @@ -105,11 +97,8 @@ export function outletHelper(params, hash, options, env) {
);
}

viewClass = viewName ? this.container.lookupFactory(viewFullName) : hash.viewClass || OutletView;

hash.currentViewBinding = '_view.outletSource._outlets.' + property;

viewClass = viewName ? this.container.lookupFactory(viewFullName) : hash.viewClass || this.container.lookupFactory('view:-outlet');
hash._outletName = property;
options.helperName = options.helperName || 'outlet';

return env.helpers.view.helperFunction.call(this, [viewClass], hash, options, env);
}
Loading

0 comments on commit e047a91

Please sign in to comment.