-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Public Router service MVP #14805
Public Router service MVP #14805
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
/** | ||
@module ember | ||
@submodule ember-routing | ||
*/ | ||
|
||
import { | ||
Service, | ||
readOnly | ||
} from 'ember-runtime'; | ||
import { get } from 'ember-metal'; | ||
import RouterDSL from '../system/dsl'; | ||
|
||
/** | ||
The Router service is the public API that provides component/view layer | ||
access to the router. | ||
|
||
@public | ||
@class RouterService | ||
@category ember-routing-router-service | ||
*/ | ||
const RouterService = Service.extend({ | ||
currentRouteName: readOnly('router.currentRouteName'), | ||
currentURL: readOnly('router.currentURL'), | ||
location: readOnly('router.location'), | ||
rootURL: readOnly('router.rootURL'), | ||
|
||
/** | ||
Transition the application into another route. The route may | ||
be either a single route or route path: | ||
|
||
See [Route.transitionTo](http://emberjs.com/api/classes/Ember.Route.html#method_transitionTo) for more info. | ||
|
||
@method transitionTo | ||
@category ember-routing-router-service | ||
@param {String} name the name of the route or a URL | ||
@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 {Transition} the transition object associated with this | ||
attempted transition | ||
@public | ||
*/ | ||
transitionTo() { | ||
this.router.transitionTo(...arguments); | ||
} | ||
}); | ||
|
||
export default RouterService; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -138,6 +138,10 @@ const EmberRouter = EmberObject.extend(Evented, { | |
init() { | ||
this._super(...arguments); | ||
|
||
this.currentURL = null; | ||
this.currentRouteName = null; | ||
this.currentPath = null; | ||
|
||
this._qpCache = new EmptyObject(); | ||
this._resetQueuedQueryParameterChanges(); | ||
this._handledErrors = dictionary(null); | ||
|
@@ -629,6 +633,7 @@ const EmberRouter = EmberObject.extend(Evented, { | |
|
||
let doUpdateURL = function() { | ||
location.setURL(lastURL); | ||
set(emberRouter, 'currentURL', lastURL); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should actually do this in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can do that. My thought was I wanted to keep both set calls in the same place, but these methods are so closely related that it's find either way. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, so it turns out that setting this after a scheduled call is what's fixing this issue. So, it looks like I have to leave it where it is. |
||
}; | ||
|
||
router.updateURL = function(path) { | ||
|
@@ -639,6 +644,7 @@ const EmberRouter = EmberObject.extend(Evented, { | |
if (location.replaceURL) { | ||
let doReplaceURL = function() { | ||
location.replaceURL(lastURL); | ||
set(emberRouter, 'currentURL', lastURL); | ||
}; | ||
|
||
router.replaceURL = function(path) { | ||
|
@@ -1291,9 +1297,11 @@ function updatePaths(router) { | |
|
||
let path = EmberRouter._routePath(infos); | ||
let currentRouteName = infos[infos.length - 1].name; | ||
let currentURL = router.get('location').getURL(); | ||
|
||
set(router, 'currentPath', path); | ||
set(router, 'currentRouteName', currentRouteName); | ||
set(router, 'currentURL', currentURL); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would think this change is not needed since we are setting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought this too, initially. It turns out that there's a different codepath whether you're visiting vs. transitioning, and we need this update to occur in both places. I think this surfaces part of @krisselden's concerns, where we want to take advantage, while we have the engine out on the floor, to refactor this to make it cleaner and more consistent. |
||
|
||
let appController = getOwner(router).lookup('controller:application'); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should add the
@category ember-routing-router-service
flag here too (so that while the feature is pending it isn't listed in emberjs.com/api).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!