Decouple controller lookup from URL generation. #14980
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
Currently, calling `emberRouter.routerMicrolib.generate('somethign');` will
force both `something` route and `something` controller to be looked up.
We do this so that we can properly handle any query params within that
route/controller pair.
The `ember-routing-router-service` work is adding a `urlFor` method, that
will ultimately allow users to generate urls without forcing the controllers
to be looked up eagerly. Unfortunately, at the moment the eager controller
lookup is completely entangled with the `getHandler` function that we provide
to the router microlib.
This commit removes the forced eager evaluation of `_qp` on a route within the
`getHandler` function, while still supporting the current implementation of
`{{link-to}}` (which still utilizes the eager `_qp` evaluation to generate
`href`'s).
This was referenced Mar 3, 2017
scalvert
reviewed
Mar 3, 2017
| @@ -609,7 +609,6 @@ const EmberRouter = EmberObject.extend(Evented, { | |||
| } | |||
|
|
|||
| handler._setRouteName(routeName); | |||
| handler._populateQPMeta(); | |||
There was a problem hiding this comment.
So you've removed this to enable lazy evaluating of qp meta, but where is this lazy evaluation happening? How is this backwards compatible?
There was a problem hiding this comment.
@scalvert - Great question, general reasons below:
- if you look at
_populateQPMeta()'s definition, all it does is eagerlythis.get('_qp')and save it off into the bucket cache, however nothing ever looks it up!!! - Things like
Ember.Router#_prepareQueryParams(used by{{link-to,transitionTo, etc) are callingroute._getQPMeta()which doesreturn route.get('_qp')(I'm guessing that at some point it was returning from the bucket cache, but it does not do this any longer). This still works (regardless of_populateQPMeta()being called first).
|
Good investigative work on this @rwjblue! |
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.
Currently, calling
emberRouter.routerMicrolib.generate('somethign');will force bothsomethingroute andsomethingcontroller to be looked up. We do this so that we can properly handle any query params within that route/controller pair.The
ember-routing-router-servicework is adding aurlFormethod, that will ultimately allow users to generate urls without forcing the controllers to be looked up eagerly. Unfortunately, at the moment the eager controller lookup is completely entangled with thegetHandlerfunction that we provide to the router microlib.This commit removes the forced eager evaluation of
_qpon a route within thegetHandlerfunction, while still supporting the current implementation of{{link-to}}(which still utilizes the eager_qpevaluation to generatehref's).Some work is needed in ember-engines to support this PR, which I am working on now...
I did emberobserver.com searches for the following:
TODO: