[REFACTOR] Lazily reify router’s location #10483
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -103,8 +105,12 @@ export default EmberObject.extend({ | |||
| this.registry.register('-application-instance:main', this, { instantiate: false }); | |||
| }, | |||
|
|
|||
| router: computed(function() { | |||
| return this.container.lookup('router:main'); | |||
| }), | |||
|
although maybe out of scope, the pattern of overriting an attribute like this is crappy, can we move away from it? |
|
@stefanpenner What do you mean overwriting an attribute? |
|
The location property is rewritten from string to instance |
|
@stefanpenner Yes, this pattern is terrible. Unfortunately it is used in almost every Ember app. We should not introduce new APIs that use this pattern but I think we're shackled with it for the time being. |
when do people access |
Users can override a router’s location by specifying its `location`
property as a string. For example, to change the router from the “auto”
location to the “none” location, users can do the following:
```js
// app/router.js
export default Ember.Router.extend({
location: 'none'
});
```
Previously, the reification of the string value into a concrete Location
implementation happened at router creation time.
This immediate reification made it difficult to make changes to the
router configuration, since it had to be done at creation time. In
general, classes that require configuration to be set at creation time
are unwieldy to use with the container, since the container itself
manages creation.
For example, in the case of the Application's `visit()` API, the
application instance would like to override the router to use the
`none` location.
When reification was at creation time, the router was created with the
wrong location. Before the default could be overridden, the router would
try to set up things like listeners on the browser's address bar, which
would cause an exception to be thrown in Node environments where there
is no notion of a URL.
In this commit, reifying and setting up the location are deferred until
the last possible moment, when routing starts (either by calling
`startRouting()`, which starts the app at the browser's current URL, or
by calling `handleURL()`, which starts the app at a provided URL). This
allows the application to detect if it is in autoboot mode or not, and
override the router's location before routing begins.
tomdale
added a commit
that referenced
this pull request
Mar 5, 2015
[REFACTOR] Lazily reify router’s location
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Users can override a router’s location by specifying its
locationproperty as a string. For example, to change the router from the “auto” location to the “none” location, users can do the following:Previously, the reification of the string value into a concrete Location implementation happened at router creation time.
This immediate reification made it difficult to make changes to the router configuration, since it had to be done at creation time. In general, classes that require configuration to be set at creation time are unwieldy to use with the container, since the container itself manages creation.
For example, in the case of the Application's
visit()API, the application instance would like to override the router to use thenonelocation.When reification was at creation time, the router was created with the wrong location. Before the default could be overridden, the router would try to set up things like listeners on the browser's address bar, which would cause an exception to be thrown in Node environments where there is no notion of a URL.
In this commit, reifying and setting up the location are deferred until the last possible moment, when routing starts (either by calling
startRouting(), which starts the app at the browser's current URL, or by callinghandleURL(), which starts the app at a provided URL). This allows the application to detect if it is in autoboot mode or not, and override the router's location before routing begins.