Skip to content
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

Determine how to handle query params. #6

Closed
rwjblue opened this issue Dec 22, 2015 · 13 comments
Closed

Determine how to handle query params. #6

rwjblue opened this issue Dec 22, 2015 · 13 comments

Comments

@rwjblue
Copy link
Member

rwjblue commented Dec 22, 2015

{{link-to 'some-qp-route' (query-param foo='otherThing')}} will need to know the default values to properly generate /some-qp-route or /some-qp-route?foo=otherThing.

I am not sure how exactly to tackle this, but I wanted to notate the concern so we can circle around once we have more fleshed out.

@rwjblue
Copy link
Member Author

rwjblue commented Dec 26, 2015

After further thought, I think that we can likely leave this alone. The following scenarios should all work, and only one is slightly non-optimal:

  • {{link-to 'some-qp-route' normalModelParams}} -- This should work fine, the default QP values would be used.
  • {{link-to 'some-qp-route' normalModelParams (query-param foo='bar'}} -- This should work fine, but would show the link as /some-qp-route?foo=bar even if bar was the default value of foo.

@nathanhammond
Copy link
Contributor

Assuming we want to handle default values we would need to pull all of that out similarly to the route-serializer story. See #22. Other than that I can't tell anything about query params that needs to be available before the engine is loaded.

However, we don't currently have the concept for linking into and out of an engine nailed down. See #57. I know @trentmwillis is working on an RFC based upon a lot of sketching on a whiteboard yesterday.

@rwjblue
Copy link
Member Author

rwjblue commented Feb 6, 2016

@nathanhammond - I believe the way that Ember currently handles default values (removing a given QP from the query string if it is the default value) is basically wrong, and that we should always include the default value. If that is the route we go with, then no real changes will be needed here (as mentioned in #6 (comment)).

@cibernox
Copy link

cibernox commented Feb 6, 2016

@rwjblue I kind of like that if the query params matches the default value the url looks clean. Reading /admin/users is nicer than /admin/users?sort=id&page=1&filter=.

@trentmwillis
Copy link
Member

@cibernox I think what @rwjblue is proposing though would still get you the clean URL if you just don't pass any query params (which should be the same as passing defaults). I think it makes sense that if explicit query params were set they should be reflected in the URL.

@nathanhammond
Copy link
Contributor

@rwjblue @cibernox I believe that in a single-page-application we're more likely to set the value for a query param in some global place and just have it always be present on the link, so we're more likely to end up with these values specified.

Otherwise, to avoid passing in default values, we would end up doing weird things with subexpressions to try to lisp our way out of the problem, but we wouldn't know what the child engine's defaults were. See: emberjs/rfcs#113

Note that I don't particularly care either way.

@rwjblue
Copy link
Member Author

rwjblue commented Feb 7, 2016

@cibernox I think what @rwjblue is proposing though would still get you the clean URL if you just don't pass any query params (which should be the same as passing defaults). I think it makes sense that if explicit query params were set they should be reflected in the URL.

Yes, exactly.

@trentmwillis
Copy link
Member

@nathanhammond I don't actually believe that would be true. The times during which you always have query params present on links are typically when you are already internal to the route with query params (e.g., on a filtering page within search results) or if you are transitioning into a route with non-default values (e.g., navigating to a specific page of paginated users).

I would argue that in those cases, you would want the query params to be present in the url even if you happen to be going to default values so that you can definitively tell the state you're in. Kind of like if you go from /users?page=2 back to the first page it is reasonable to see /users?page=1, even if it was /users when you first visited it.

In my experience, the primary cases where you explicitly want default values are when you are external to the route with the query params (e.g., transitioning into the first page of a paginated route that lists users), in that case you are unlikely to actually have query params set on the link.

@nathanhammond
Copy link
Contributor

@sethkinast you're being tagged in as a person who might actually have strong opinions on this topic. 😄 It seems like the rest of us are mostly just bikeshedding and don't really care either way.

@sethkinast
Copy link

From the philosophical point of view, I believe in link permanence and so I also agree that Ember rewriting links when query params are default is wrong.

Take something like a sales reporting dashboard, where you could filter by different criteria such as date, and the default is today's date. I would expect to be able to copy the URL of a page or a link to a page at any time and always be able to return to that page, even if the underlying application state or assumptions have changed since I copied the link, because I should always be able to reconstruct the entire relevant page state from a URL in a well-designed application.

So I send an email about "our sales for today," and include a link to what I think is the sales dashboard, because I filtered by today's date. Someone clicks the link tomorrow, and gets a different application state that what I linked, because today's date is simply the default.

From a more pragmatic point of view, I think it's weird that a foreign object I'm linking to (an engine) could somehow affect the link that my app generates, especially since the engine can change underneath me over time. So maybe today I have a test that asserts that a generated URL is /foo?bar=true, and one day it starts failing because my foo engine now has bar: true by default; what I really care about, though, is that a link to the correct foo engine state is being created.

As additional color, I think that the general design that a single controller exclusively owns one or more query params is just a flawed design to begin with. Query params are simply a read-only snapshot of application state and there shouldn't be a reason that they shouldn't be accessible at all nested levels of my app. The contrapositive is that there shouldn't be a need to bind query params to mutable controller params to be able to observe them.

If you take these assertions to their logical conclusion that multiple entities in your application have an interest in the same query param (which we have extensive issues dealing with today), it follows that there is no longer any such thing as a "default" state because different entities might have different defaults for that value.

@dgeb
Copy link
Member

dgeb commented Jun 14, 2016

@sethkinast The core team has decided to move away from the concept of default query param values at this weekend's face-to-face meeting. See emberjs/rfcs#95 (comment)

@villander
Copy link
Member

@dgeb can you close this?

@villander
Copy link
Member

@rwjblue @dgeb can we close this, please?

@dgeb dgeb closed this as completed Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants