-
-
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
Query-Params Lifecycle is Unpredictable, Inconsistent #14785
Comments
I originally opened this as emberjs/rfcs#192, but @rwjblue suggested I switch to an Issue until we have some sort of implementation plan, which will then become the RFC. See also http://stackoverflow.com/q/35192211/1190, which works in an action handler, but not in a route hook like |
@jamesarosen Nice write up :) I'm curious about where this issue needs to be tracked, Did @rwjblue suggest and Ember.js/issue or and RFC/issue ? See, See CONTRIBUTING.md#requesting-a-feature which recommends creating an RFC Issue to request a feature/enhancement (no need for a PR with RFC, only an issue in the repo) |
As I said above,
I assumed he meant an emberjs Issue, but I guess it could have been an RFC Issue. There are many RFC Issues, but the RFC docs don't describe their use-case. From CONTRIBUTING:
I think that's exactly the case for this. I wish that information were in the RFC README as well. I had no idea. |
@jamesarosen @pixelhandler can y'all take care of improving CONTRIBUTING and moving this issue to an rfcs repo issue? Thanks! |
Moved to emberjs/rfcs#196 |
Overview
The guides for query-params discuss at length how to set query-params, including customizations. They do not, however, formally specify the lifecycle of query-params. Specifically, the following are left unspecified:
This ember-twiddle demonstrates some oddities:
/
directly (by typing it into the location bar and pressing Enter). The value from the Controller is"defaultValue"
, which is the value the Controller specifies as the default. The value fromtransition.queryParams.q
isnull
. In the Route'smodel
hook, the Controller's value is the default value./?q=something
directly. The value from the Controller is"something"
. The value oftransition.queryParams.q
is stillnull
. (I think this particular issue is related to the application running in an iframe.) The value that the Route'smodel
hook sees from the controller is"defaultValue"
, meaning the query-param hasn't been synced to the Controller./?q=forbidden
directly. This acts just like the/?q=something
case. But if you click the/?q=something
link in the footer and then the/?q=forbidden
link, it changes. TheserializeQueryParam
kicks in, changing"forbidden"
tonull
, which affects the location and the value from the Controller. This shows thatserializeQueryParam
is invoked on every transition after the first./?v=123
directly. Becausev
isn't declared as a query-param, Ember strips it from the URL. This shows that invalid query-params, but not valid ones are re-serialized during the first transition.Recommendations
I do not yet have a clear, holistic implementation in mind. I think the following requirements are a bare minimum:
transition.queryParams
are available and what effects modifications to that object will have.beforeModel
" or "betweenafterModel
andsetupController
") query-params will be synced between the Location and the Controller.beforeModel/model/afterModel/setupController
flow.Use Cases
route:application#model
). If a non-admin goes to/some-path?userId=3456
, the route should clear that query-param after it is established the user lacks permissions./some-path
and the application generates a split-testing ID on-the-fly, then sets it as a query-param. The Location should immediately reflect this ID. (Having this ID in the URL could make it easier when handling support tickets since it would be clear which version of the interface they were using.)Existing Work
ember-query-params attempts to move some of the query-param logic into a service. This is good for encapsulating logic around validating, filtering, and manipulating query-params, but doesn't address the lifecycle issues.
Unresolved questions
The text was updated successfully, but these errors were encountered: