Refactor context handling for States and Routes

emberjs · Jun 25, 2012 · 3d55878 · 3d55878
1 parent 260b1b4
commit 3d55878
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 117 deletions.
diff --git a/packages/ember-routing/lib/routable.js b/packages/ember-routing/lib/routable.js
@@ -136,8 +136,21 @@ Ember.Routable = Ember.Mixin.create({
     string property.
   */
   routeMatcher: Ember.computed(function() {
-    if (get(this, 'route')) {
-      return Ember._RouteMatcher.create({ route: get(this, 'route') });
+    var route = get(this, 'route');
+    if (route) {
+      return Ember._RouteMatcher.create({ route: route });
+    }
+  }).cacheable(),
+
+  /**
+    @private
+
+    Check whether the route has dynamic segments
+  */
+  isDynamic: Ember.computed(function() {
+    var routeMatcher = get(this, 'routeMatcher');
+    if (routeMatcher) {
+      return routeMatcher.identifiers.length > 0;
     }
   }).cacheable(),
 
@@ -288,7 +301,7 @@ Ember.Routable = Ember.Mixin.create({
 
     Ember.assert("Could not find state for path " + path, !!state);
 
-    var object = state.deserialize(manager, match.hash) || {};
+    var object = state.deserialize(manager, match.hash);
     manager.transitionTo(get(state, 'path'), object);
     manager.send('routePath', match.remaining);
   },

diff --git a/packages/ember-routing/lib/route_matcher.js b/packages/ember-routing/lib/route_matcher.js
@@ -40,7 +40,7 @@ Ember._RouteMatcher = Ember.Object.extend({
 
       return {
         remaining: path.substr(match[0].length),
-        hash: hash
+        hash: identifiers.length > 0 ? hash : null
       };
     }
   },

diff --git a/packages/ember-routing/tests/routable_test.js b/packages/ember-routing/tests/routable_test.js
@@ -39,11 +39,11 @@ test("a RouteMatcher matches routes", function() {
 
   match = matcher.match('foo');
   equal(match.remaining, "");
-  deepEqual(match.hash, {});
+  equal(match.hash, null);
 
   match = matcher.match('foo/bar');
   equal(match.remaining, "/bar");
-  deepEqual(match.hash, {});
+  equal(match.hash, null);
 
   match = matcher.match('bar');
   equal(match, undefined);
@@ -424,7 +424,7 @@ var locationStub = {
   setURL: Ember.K
 };
 var expectURL = function(url) {
-  equal(url, formatURLArgument, "should invoke formatURL with URL "+url);
+  equal(formatURLArgument, url, "should invoke formatURL with URL "+url);
 };
 
 test("urlFor returns an absolute route", function() {

diff --git a/packages/ember-states/lib/state.js b/packages/ember-states/lib/state.js
@@ -88,7 +88,7 @@ Ember.State = Ember.Object.extend(Ember.Evented,
       }
     }
 
-    set(this, 'routes', {});
+    set(this, 'pathsCache', {});
   },
 
   /** @private */

diff --git a/packages/ember-states/lib/state_manager.js b/packages/ember-states/lib/state_manager.js
@@ -513,9 +513,9 @@ Ember.StateManager = Ember.State.extend(
     return possible;
   },
 
-  findStatesByRoute: function(state, route) {
-    if (!route || route === "") { return undefined; }
-    var r = route.split('.'),
+  findStatesByPath: function(state, path) {
+    if (!path || path === "") { return undefined; }
+    var r = path.split('.'),
         ret = [];
 
     for (var i=0, len = r.length; i < len; i++) {
@@ -537,122 +537,110 @@ Ember.StateManager = Ember.State.extend(
     return this.transitionTo.apply(this, arguments);
   },
 
-  pathForSegments: function(array) {
-    return Ember.ArrayPolyfills.map.call(array, function(tuple) {
-      Ember.assert("A segment passed to transitionTo must be an Array", Ember.typeOf(tuple) === "array");
-      return tuple[0];
-    }).join(".");
-  },
-
-  transitionTo: function(name, context) {
+  transitionTo: function(path, context) {
     // 1. Normalize arguments
     // 2. Ensure that we are in the correct state
     // 3. Map provided path to context objects and send
     //    appropriate transitionEvent events
 
-    if (Ember.empty(name)) { return; }
-
-    var segments, explicitSegments;
-
-    if (Ember.typeOf(name) === "array") {
-      segments = [].slice.call(arguments);
-      explicitSegments = true;
-    } else {
-      segments = [[name, context]];
-      explicitSegments = false;
-    }
+    if (Ember.empty(path)) { return; }
 
-    var path = this.pathForSegments(segments),
+    var contexts = context ? Array.prototype.slice.call(arguments, 1) : [],
         currentState = get(this, 'currentState') || this,
-        state = currentState,
-        newState,
+        resolveState = currentState,
         exitStates = [],
         enterStates,
-        resolveState,
-        setupContexts = [];
+        targetState,
+        state,
+        initialState;
+
+    // Is the cache useful anymore?
 
-    if (state.routes[path]) {
+    if (currentState.pathsCache[path]) {
       // cache hit
 
-      var route = state.routes[path];
-      exitStates = route.exitStates;
-      enterStates = route.enterStates;
-      state = route.futureState;
-      resolveState = route.resolveState;
+      var cachedPath = currentState.pathsCache[name];
+      exitStates = cachedPath.exitStates;
+      enterStates = cachedPath.enterStates;
+      resolveState = cachedPath.resolveState;
+      targetState = cachedPath.targetState;
     } else {
       // cache miss
 
-      newState = this.findStatesByRoute(currentState, path);
+      enterStates = this.findStatesByPath(currentState, path);
 
-      while (state && !newState) {
-        exitStates.unshift(state);
+      while (resolveState && !enterStates) {
+        exitStates.unshift(resolveState);
 
-        state = get(state, 'parentState');
-        if (!state) {
-          newState = this.findStatesByRoute(this, path);
-          if (!newState) { return; }
+        resolveState = get(resolveState, 'parentState');
+        if (!resolveState) {
+          enterStates = this.findStatesByPath(this, path);
+          if (!enterStates) { return; }
         }
-        newState = this.findStatesByRoute(state, path);
+        enterStates = this.findStatesByPath(resolveState, path);
       }
 
-      resolveState = state;
+      while (enterStates.length > 0 && enterStates[0] === exitStates[0]) {
+        resolveState = enterStates.shift();
+        exitStates.shift();
+      }
 
-      enterStates = newState.slice(0);
-      exitStates = exitStates.slice(0);
+      currentState.pathsCache[name] = {
+        exitStates: exitStates,
+        enterStates: enterStates,
+        resolveState: resolveState
+      };
+    }
 
-      if (enterStates.length > 0) {
-        state = enterStates[enterStates.length - 1];
+    var matchedContexts = [],
+        stateIdx = enterStates.length-1;
+    while (contexts.length > 0) {
+      if (stateIdx >= 0) {
+        state = enterStates[stateIdx--];
+      } else {
+        state = enterStates[0] ? get(enterStates[0], 'parentState') : resolveState;
+        if (!state) { throw "Cannot match all contexts to states"; }
+        enterStates.unshift(state);
+        exitStates.unshift(state);
+      }
 
-        var initialState;
-        while(initialState = get(state, 'initialState')) {
-          state = getPath(state, 'states.'+initialState);
-          enterStates.push(state);
-        }
+      var useContext = context && (!get(state, 'isRoutable') || get(state, 'isDynamic'));
+      matchedContexts.unshift(useContext ? contexts.pop() : null);
+    }
 
-        while (enterStates.length > 0) {
-          if (enterStates[0] !== exitStates[0]) { break; }
+    if (enterStates.length > 0) {
+      state = enterStates[enterStates.length - 1];
 
-          var newContext;
-          if (explicitSegments) {
-            var segmentIndex = segments.length - enterStates.length;
-            newContext = segments[segmentIndex][1];
-          } else if (enterStates.length === 1) {
-            newContext = context;
-          }
+      while(true) {
+        initialState = get(state, 'initialState') || 'start';
+        state = getPath(state, 'states.'+initialState);
+        if (!state) { break; }
+        enterStates.push(state);
+      }
 
-          if (newContext) {
-            if (newContext !== this.getStateMeta(enterStates[0], 'context')) { break; }
-          }
+      while (enterStates.length > 0) {
+        if (enterStates[0] !== exitStates[0]) { break; }
 
-          enterStates.shift();
-          exitStates.shift();
+        if (enterStates.length === matchedContexts.length) {
+          if (this.getStateMeta(enterStates[0], 'context') !== matchedContexts[0]) { break; }
+          matchedContexts.shift();
         }
 
-        if (enterStates.length > 0) {
-          setupContexts = Ember.EnumerableUtils.map(enterStates, function(state, index) {
-                            return [state, explicitSegments ? segments[index][1] : context];
-                          });
-        }
+        resolveState = enterStates.shift();
+        exitStates.shift();
       }
-
-      currentState.routes[path] = {
-        exitStates: exitStates,
-        enterStates: enterStates,
-        futureState: state,
-        resolveState: resolveState
-      };
     }
 
-    this.enterState(exitStates, enterStates, state);
-    this.triggerSetupContext(setupContexts);
+    this.enterState(exitStates, enterStates, enterStates[enterStates.length-1] || resolveState);
+    this.triggerSetupContext(enterStates, matchedContexts);
   },
 
-  triggerSetupContext: function(segments) {
-    arrayForEach.call(segments, function(tuple) {
-      var state = tuple[0],
-          context = tuple[1];
+  triggerSetupContext: function(enterStates, contexts) {
+    var offset = enterStates.length - contexts.length;
+    Ember.assert("More contexts provided than states", offset >= 0);
 
-      state.trigger(get(this, 'transitionEvent'), this, context);
+    arrayForEach.call(enterStates, function(state, idx) {
+      state.trigger(get(this, 'transitionEvent'), this, contexts[idx-offset]);
     }, this);
   },
 
@@ -681,27 +669,6 @@ Ember.StateManager = Ember.State.extend(
       state.trigger('enter', stateManager);
     });
 
-    var startState = state,
-        enteredState,
-        initialState = get(startState, 'initialState');
-
-    if (!initialState) {
-      initialState = 'start';
-    }
-
-    while (startState = get(get(startState, 'states'), initialState)) {
-      enteredState = startState;
-
-      if (log) { Ember.Logger.log("STATEMANAGER: Entering " + get(startState, 'path')); }
-      startState.trigger('enter', stateManager);
-
-      initialState = get(startState, 'initialState');
-
-      if (!initialState) {
-        initialState = 'start';
-      }
-    }
-
-    set(this, 'currentState', enteredState || state);
+    set(this, 'currentState', state);
   }
 });
diff --git a/packages/ember-states/tests/state_manager_test.js b/packages/ember-states/tests/state_manager_test.js
@@ -553,11 +553,11 @@ test("multiple contexts can be provided in a single transitionTo", function() {
     });
   });
 
-  stateManager.transitionTo(['planters', { company: true }], ['nuts', { product: true }]);
+  stateManager.transitionTo('planters.nuts', { company: true }, { product: true });
 });
 
 test("transitionEvent is called for each nested state", function() {
-  expect(4);
+  expect(8);
 
   var calledOnParent = false,
       calledOnChild = true;
@@ -569,7 +569,7 @@ test("transitionEvent is called for each nested state", function() {
       planters: Ember.State.create({
         setup: function(manager, context) {
           calledOnParent = true;
-          equal(context, 'context', 'parent gets context');
+          ok(!context, 'single context is not called on parent');
         },
 
         nuts: Ember.State.create({
@@ -586,4 +586,16 @@ test("transitionEvent is called for each nested state", function() {
 
   ok(calledOnParent, 'called transitionEvent on parent');
   ok(calledOnChild, 'called transitionEvent on child');
+
+  // repeat the test now that the path is cached
+
+  stateManager.transitionTo('start');
+
+  calledOnParent = false;
+  calledOnChild = false;
+
+  stateManager.transitionTo('planters.nuts', 'context');
+
+  ok(calledOnParent, 'called transitionEvent on parent');
+  ok(calledOnChild, 'called transitionEvent on child');
 });