Use route name instead of path when saving the query params in the cache.

[cah-brian-gantzler]

For routes that have dynamic segments, the cache gets filled because the query params are store for each value of the dynamic segment. I would think that params should be tracked at the route level, not for each value of the dynamic segment.

Or should this be left as an optional feature that could be disabled. Should there be a config that say track params at Route or Path?

Pretty sure the original query params in the ember controller are at the route only

[NullVoxPopuli]

that would mean query params are shared among all route names, regardless of dynamic segments, yeah?

[cah-brian-gantzler]

It would mean for path user/1/edit?showFull=true and path user/2/edit?showFull=true that showFull is tracked once for the route user/{:id}/edit and not have a distinct value for user 1 and user 2. There would only be one record in the cache not two (or as many users as were visited)

[NullVoxPopuli]

gotchya -- 🤔

So, this would be a configured somewhere?
Thoughts as to where makes the most sense?

[cah-brian-gantzler]

I think the most sense is they are route based only. That how the current query params work on controller pretty sure. Allowing for a config that tracks at path may be more confusing than its worth, but I could see in the right context that being useful. However it would be all or nothing, it would just add to confusion to allow some routes to be at the route and other routes to respect path.

I didnt want to take away a feature that might be useful, but doing just route name would be the simplest and consistent with I think how people think they work now.

[cah-brian-gantzler]

If we were to allow both and config it, would assume it would be in the enironment/config.js

[cah-brian-gantzler]

Where did we land on this? I vote for changing from Path to RouteName

[NullVoxPopuli]

I'd like to see some highlevel proposals for how to configure each behavior, tradeoffs, what happens in each config, etc

like,

@queryParam({ 
  stickTo: 'path', // 'route', 'application'
}) a;

could be an option

[cah-brian-gantzler]

I rewrote this several times but it seems to be rambling thought, bear with me, sorry. (maybe should get on discord sometime and discuss)

How do the current controller query params work? Given

      this.route('users', function () {
        this.route('new');
        this.route('edit', { path: ':id/edit' });
      });

A query param defined on users will appear on index, new and edit. A query param defined on new will only be present there. Is that correct? With the current implementation a param defined on /user will not be present on /user/new because the path /user is different the /user/new. Is that desired? Would also work the same if changed to route. Should we be combining /user and /user/new query params? Walking the route user.edit would be easier than /user/1/edit with /user/1 with /user ( /user/1 is actually invalid isnt it?)

Path and route are the same thing when there are no dynamic segments. They are only different when there is a dynamic segment present. Path does not really communicate that nuance, maybe dynamic

For your suggested syntax, if you create a query param that says route and another that says path (in components) and both are present on the screen at the same time, if the url has a dymanic segment, we would have to store then separately but combine route with the correct path to display (as mentioned above)

so when you are on /route/1/edit for example, route param A will have a value of 1 and path param B will have a value of 2.
Then /route/2/edit route param A will still be 1, only one value, but path param B could be 7.

Is that going to be confusing or hard to hunt down that there is only one value of A but a multitude of values to B.

I think it comes down to it would the average developer understand there is a difference between path (/route/1/edit) and route (/route/{:id}/edit. Should there be a difference when it comes to query params.

This can be used in a component which is not tied to a route. If a component is in one hbs and the other component is in another that is in an {{outlet}} should they know that and be treated differently?

Proposals:

1. No config, only do route, most closely mirrors how query params in controllers work (although I think we have to add walking up the route and combining the params)
2. Config in environment.js that says path or route, all query params act the same (basically what you have now with route removing the uniqueness of the dynamic segment)(still need to do the combining)
3. The syntax you proposed, which allows for a lot of flexibility but I think some overlapping responsibilities that would lead to errors.
4. More complex configs that allow for greater flexibility but even more difficulty in understanding.

Application was an interesting add, but I think thats the same as defining an @QueryParam on the application controller or in a component used in the application hbs. (but with combining so it only doesnt show on /), or worse, stored over and over again in the cache for every visited path which would lead to different values.

Until I did all this thinking I thought this was pretty cut an dried, but guessing thats why they are still on controllers the way they are until all these kinds of questions are answered.

[NullVoxPopuli]

is different the /user/new. Is that desired?

It could be -- I've had needs for both the existing behavior and what you're describing.

Should we be combining /user and /user/new query params?

I think it makes sense to be able to declare query params as sticky to support this behavior

Is that going to be confusing or hard to hunt down that there is only one value of A but a multitude of values to B.

I don't understand what you're trying to say with this :(

Should there be a difference when it comes to query params.

it must be configurable, ya know? all scenarios are desirable

same as defining an @QueryParam on the application controller

correct -- this ties in to this RFC: emberjs/rfcs#715

or worse, stored over and over again in the cache for every visited path which would lead to different values.

the application settings would explicitly not do this

but guessing thats why they are still on controllers the way they are until all these kinds of questions are answered.

yeah, it's kinda why there has been no movement on the RFCs -- we need every option / pattern / scenario / use case to be supported, and we don't exactly know what the existing behavior is -- which is what makes migrating hard.

re: your proposals, I kinda feel it'd be best to have query param settings on routes (kinda like today, but also specifying sticky behavior, optionally (de)serialization (so that way the decorators didn't have to)). I think the recent addition of defaultValue and initial value interplay is a bit confusing, so I think we'd probably want to resolve that as well.
I don't think we want config in environment.js because that is a shared file, and we want these sorts of settings to be available within split routes without additional build time transformations.

Thanks for asking all the questions! It's important we explore this stuff

[cah-brian-gantzler]

This kinda just waned off, although I have not used ember-query-params to actually set params and this did not really work the way I expected it to and went back to doing the way it says to in the guides.

Should we have a more indepth discussion on how this should maybe work? or just close this as so much time has gone by

[NullVoxPopuli]

idk -- I think it kind of depends on what a person's needs are.

For me, I had a specific use case (what was implemented) -- you had another!

[cah-brian-gantzler]

Earlier in this thread I had expected queryParams to be a route level value and was recently surprised to learn that is not the default as stated here https://guides.emberjs.com/release/routing/query-params/#toc_sticky-query-param-values where it states the values are specific to the model. Had no idea. You can make then route specific by setting the scope value to controller. I dont think this nuance is represented in this addon.

The other real weird thing here is that the decorator @QueryParam can define the query param, so if you put it in two different components (cause they each need access to the value) and both components are on the screen at the same time, two different defaults could be registered at the same time (two different serializer/deserialize methods), I assume the last one instantiated wins. If a component is then hidden, is its definition removed and the other takes precedence. Been a while since I looked at this code so will have to look again.

Where are we with https://github.com/NullVoxPopuli/rfcs/blob/router-computed-query-params/text/0000-router-service-computed-query-params.md as that may help a lot. We really need one central place to ensure the query params are defined once, and the decorators could be used to read/write them.