-
-
Notifications
You must be signed in to change notification settings - Fork 405
RouteInfo Metadata #398
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
RouteInfo Metadata #398
Changes from all commits
b82b4a8
526cfbb
f469a74
82e7e81
ade9f97
2c3c728
7aa2971
ad67775
42ed7fb
c191ee8
36795db
fa93479
4a56424
bfed6f3
2677e9d
3d2630b
64b0f5a
caa8d87
4482746
9124f7b
653fd9c
17b6ee2
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,308 @@ | ||
| - Start Date: 2018-11-02 | ||
| - RFC PR: https://github.com/emberjs/rfcs/pull/398 | ||
| - Ember Issue: (leave this empty) | ||
|
|
||
| # RouteInfo MetaData | ||
|
|
||
| ## Summary | ||
|
|
||
| The RFC introduces the ability to associate application specific metadata with its corresponding `RouteInfo` object. This also adds a `metadata` field to `RouteInfo`, which will be the return value of `buildRouteInfoMetadata` for its corresponding `Route`. | ||
|
|
||
| ```js | ||
| // app/route/profile.js | ||
| import Route from '@ember/routing/route'; | ||
| import { inject as service } from '@ember/service'; | ||
|
|
||
| export default Route.extend({ | ||
chadhietala marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| user: inject('user'), | ||
| buildRouteInfoMetadata() { | ||
| return { | ||
| trackingKey: 'page_profile', | ||
| user: { | ||
| id: this.user.id, | ||
| type: this.user.type | ||
| } | ||
| } | ||
| } | ||
| // ... | ||
| }); | ||
| ``` | ||
|
|
||
| ```js | ||
| // app/services/analytics.js | ||
| import Service, { inject } from '@ember/service'; | ||
|
|
||
| export default Service.extend({ | ||
| router: inject('router'), | ||
| init() { | ||
| this._super(...arguments); | ||
| this.router.on('routeDidUpdate', (transition) => { | ||
| let { to, from } = transition; | ||
| let fromMeta = from.metadata; | ||
| let toMeta = to.metadata; | ||
| ga.sendEvent('pageView', { | ||
| from: fromMeta, | ||
| to: toMeta, | ||
| timestamp: Date.now(), | ||
| }) | ||
| }) | ||
| }, | ||
| // ... | ||
| }); | ||
| ``` | ||
|
|
||
| ## Motivation | ||
|
|
||
| While the `RouteInfo` object is sufficient in providing developers metadata about the `Route` itself, it is not sufficient in layering on application specific metadata about the `Route`. This metadata could be anything from a more domain-specific name for a `Route`, e.g. `profile_page` vs `profile.index`, all the way to providing contextual data when the `Route` was visited. | ||
|
|
||
| This metadata could be used for more pratical things like updating the `document.title`. | ||
|
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 While a proposed stateless For example, let's say I have a route with a dynamic segment
Contributor
Author
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 this is actually possible with the combination of 2 Router Service. On the Router Service there is export default Route.extend({
buildRouteInfoMetadata() {
return {
title(model) {
return `Product: ${model.name}`;
}
}
}
// ...
});this.router.on('routeDidUpdate', (transition) => {
document.title = transition.to.title(this.router.currentRoute.attributes);
})
``
Contributor
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. Any time we lean on the global Maybe that is OK and unavoidable, but it's worth thinking about alternatives that make the data dependency more explicit: this.router.on('routeDidUpdate', async (transition) => {
let model = await transition.to.withAttributes();
document.title = transition.to.title(model);
})(in which I'm imagining a new
Contributor
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. There's a tradeoff here, because the simple thing introduces asynchrony, which then also invites a whole host of other issues.
Contributor
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. Wait, is there any reason
Contributor
Author
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. Yes it could receive the |
||
| Currently, addons like [Ember CLI Head](https://github.com/ronco/ember-cli-head) and [Ember CLI Document Title](https://github.com/kimroen/ember-cli-document-title) require the user to supply special metadata fields on your `Route` that will be used to update the title. This API would be a formalized place to place that metadata. | ||
|
|
||
| See the [appendix](#appendix-a) for examples. | ||
|
|
||
| ## Detailed design | ||
|
|
||
| ### `buildRouteInfoMetadata` | ||
|
Contributor
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. This is
Contributor
Author
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. Yes
Contributor
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. Would it be possible to name that something a bit shorter? i.e.
Member
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 that it’s not going to be super super common and verbosity also helps keep it clear what’s going on while also avoiding clobbering existing methods.... |
||
|
|
||
| This optional hook is intended to be used as a way of letting the routing system know about any metadata associated with the route. | ||
|
|
||
| #### `Route` Interface Extension | ||
|
|
||
| ```ts | ||
| interface Route { | ||
| // ... existing public API | ||
| buildRouteInfoMetadata(): unknown | ||
| } | ||
| ``` | ||
|
|
||
| #### Runtime Semantics | ||
|
|
||
| - **Always** called before the `beforeModel` hook is called | ||
| - **Maybe** called more than once during a transition e.g. aborts, redirects. | ||
|
|
||
| ### `RouteInfo.metadata` | ||
|
|
||
| The `metadata` optional field on `RouteInfo` will be populated with the return value of `buildRouteInfoMetadata`. If there is no metadata associated with the `Route`, the `metadata` field will be `null`. | ||
|
|
||
| ```ts | ||
| interface RouteInfo { | ||
| // ... existing public API | ||
| metadata: Maybe<unknown>; | ||
| } | ||
| ``` | ||
|
|
||
| This field will also be added to `RouteInfoWithAttributes` as it is just a super-set of `RouteInfo`. | ||
|
|
||
|
|
||
| ## How we teach this | ||
|
|
||
| We feel that this a low-level primitive that will allow existing tracking addons to encapsulate. That being said the concept here is pretty simple: What gets returned from `buildRouteInfoMetadata` becomes the value of `RouteInfo.metadata` for that `Route`. | ||
|
|
||
| The guides and tutorial should be updated to incorporate an example on how these APIs could integrate with services like Google Analytics. | ||
|
|
||
| ## Drawbacks | ||
|
|
||
| This adds an additional hook that is called during route activation, expanding the surface area of the `Route` class. | ||
| While this is true, there is currently no good way to associate application-specific metadata with a route transition. | ||
|
|
||
| ## Alternatives | ||
|
|
||
| There are numerous alternative to the proposal: | ||
|
|
||
| ### `setRouteMetadata` | ||
|
|
||
| This API would be similar to `setComponentManager` and `setModifierManager`. For example: | ||
|
|
||
| ```js | ||
| // app/route/profile.js | ||
| import Route, { setRouteMetadata } from '@ember/routing/route'; | ||
|
|
||
| export default Route.extend({ | ||
|
|
||
| init() { | ||
| this._super(...arguments); | ||
| setRouteMetadata(this, { | ||
| trackingKey: 'page_profile', | ||
| profile: { | ||
| viewing: this.userId, | ||
| locale: this.userLocale | ||
| } | ||
| }); | ||
| } | ||
| // ... | ||
| }); | ||
| ``` | ||
|
|
||
| You would then use the a `RouteInfo` to lookup the value: | ||
|
|
||
|
|
||
| ```js | ||
| // app/services/analytics.js | ||
| import { getRouteMetadata } from '@ember/routing/route'; | ||
| import Service, { inject } from '@ember/service'; | ||
| export default Service.extend({ | ||
| router: inject('router'), | ||
| init() { | ||
| this._super(...arguments); | ||
| this.router.on('routeDidUpdate', (transition) => { | ||
| let { to, from } = transition; | ||
| let { trackingKey: fromKey } = getRouteMetadata(from); | ||
| let { trackingKey: toKey } = getRouteMetadata(to); | ||
| ga.sendEvent('pageView', { | ||
| from: fromKey, | ||
| to: toKey, | ||
| timestamp: Date.now(), | ||
| }) | ||
| }) | ||
| }, | ||
| // ... | ||
| }); | ||
| ``` | ||
|
|
||
| This could work but there are two things that are confusing here: | ||
|
|
||
| 1. What happens if you call `setRouteMetadata` mutliple times. Do you clobber the existing metadata? Do you merge it? | ||
| 2. It is very odd that you would use a `RouteInfo` to access the metadata when you set it on the `Route`. | ||
|
|
||
| ### `Route.metadata` | ||
|
|
||
| This would add a special field to the `Route` class that would be copied off on to the `RouteInfo`. For example: | ||
|
|
||
| ```js | ||
| // app/route/profile.js | ||
| import Route, { setRouteMetadata } from '@ember/routing/route'; | ||
|
|
||
| export default Route.extend({ | ||
| metadata: { | ||
| trackingKey: 'page_profile', | ||
| profile: { | ||
| viewing: this.userId, | ||
| locale: this.userLocale | ||
| } | ||
| } | ||
| // ... | ||
| }); | ||
| ``` | ||
|
|
||
| The value would then be populated on `RouteInfo.metadata`. | ||
|
|
||
|
|
||
| ```js | ||
| // app/services/analytics.js | ||
| import { getRouteMetadata } from '@ember/routing/route'; | ||
| import Service, { inject } from '@ember/service'; | ||
| export default Service.extend({ | ||
| router: inject('router'), | ||
| init() { | ||
| this._super(...arguments); | ||
| this.router.on('routeDidUpdate', (transition) => { | ||
| let { to, from } = transition; | ||
| let fromMeta = from.metadata; | ||
| let toMeta = to.metadata; | ||
| ga.sendEvent('pageView', { | ||
| from: fromKey, | ||
| to: toKey, | ||
| timestamp: Date.now(), | ||
| }) | ||
| }) | ||
| }, | ||
| // ... | ||
| }); | ||
| ``` | ||
|
|
||
| This could work but there are two things that are problematic here: | ||
|
|
||
| 1. What happens to the this data if you subclass it? Do you merge or clobber the field? | ||
| 2. This is a generic property name and may conflict in existing applications | ||
|
|
||
| ### Return Metadata From `activate` | ||
|
|
||
| Today `activate` does not get called when the dynamic segments of the `Route` change, making it not well fit for this use case. | ||
|
|
||
| ## Unresolved questions | ||
|
|
||
| TBD? | ||
|
|
||
|
|
||
| ### Apendix A | ||
|
|
||
| Tracking example | ||
|
|
||
| ```js | ||
| // app/route/profile.js | ||
| import Route from '@ember/routing/route'; | ||
| import { inject } from '@ember/service'; | ||
| export default Route.extend({ | ||
| user: inject('user'), | ||
| buildRouteInfoMetadata() { | ||
| return { | ||
| trackingKey: 'page_profile', | ||
| user: { | ||
| id: this.user.id, | ||
| type: this.user.type | ||
| } | ||
| } | ||
| } | ||
| // ... | ||
| }); | ||
| ``` | ||
|
|
||
| ```js | ||
| // app/services/analytics.js | ||
| import Service, { inject } from '@ember/service'; | ||
|
|
||
| export default Service.extend({ | ||
| router: inject('router'), | ||
| init() { | ||
| this._super(...arguments); | ||
| this.router.on('routeDidUpdate', (transition) => { | ||
| let { to, from } = transition; | ||
| let fromMeta = from.metadata; | ||
| let toMeta = to.metadata; | ||
| ga.sendEvent('pageView', { | ||
| from: fromMeta, | ||
| to: toMeta, | ||
| timestamp: Date.now(), | ||
| }) | ||
| }) | ||
| }, | ||
| // ... | ||
| }); | ||
| ``` | ||
|
|
||
|
|
||
| ### Appendix B | ||
|
|
||
| Updating document.title | ||
|
|
||
| ```js | ||
| // app/route/profile.js | ||
| import Route from '@ember/routing/route'; | ||
| import { inject } from '@ember/service'; | ||
| export default Route.extend({ | ||
| user: inject('user'), | ||
| buildRouteInfoMetadata() { | ||
| return { | ||
| title: 'My Cool WebPage' | ||
| } | ||
| } | ||
| // ... | ||
| }); | ||
| ``` | ||
|
|
||
| ```js | ||
| // app/router.js | ||
| import Router from '@ember/routing/router'; | ||
|
|
||
| // ... | ||
| export default Router.extend({ | ||
| init() { | ||
| this._super(...arguments); | ||
| this.on('routeDidUpdate', (transition) => { | ||
| let { title } = transition.metadata; | ||
| document.title = title; | ||
| }); | ||
| }, | ||
| // ... | ||
| }); | ||
| ``` | ||
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.
typo: importing as the name
servicebut usinginjectbelow