Skip to content
This repository has been archived by the owner on Feb 25, 2022. It is now read-only.

Query Params vs URL Params #50

Closed
talamaska opened this issue Oct 1, 2021 · 15 comments
Closed

Query Params vs URL Params #50

talamaska opened this issue Oct 1, 2021 · 15 comments
Labels

Comments

@talamaska
Copy link

I find it a bit confusing that both are mapped to state.params. Is there a reason for that? Would you consider actually adding state.queryParams for query parameters?

@csells
Copy link
Owner

csells commented Oct 1, 2021

It's just a bag of name-value pairs that shouldn't overlap. I could think of no reason to keep both in separate spots and I prefer a smaller API surface when I can get one. Do you have a good use case for two separate collections?

@talamaska
Copy link
Author

talamaska commented Oct 5, 2021

Well, in Angular router we have both of them handled separately. In reality you could have same name params, query params obviously are those after the question mark in the end of the url, path param is part of the url could be in the middle. I know you know this. Path param could lead to a separate page/widget, while query params cannot and can be used only to provide data to any route. I mean if your making a router that should work on web, you should have them distinguished. I can't imagine any of the web frameworks dropping this separation.

@csells
Copy link
Owner

csells commented Oct 5, 2021

Correct. That's the behavior go_router provides. My question is, is there a reason to have inline and query params in separate scopes? Do the names need to overlap for any reason?

@talamaska
Copy link
Author

They don't need, but they could. I mean you could ask the Angular team why they have them separated in different collections. For me it gives more freedom, while if they are in a same collection it is an artificial / unnatural limitation of what web normally provides.

@csells
Copy link
Owner

csells commented Oct 5, 2021

It's only a limitation if there's a reason for separate scopes. Perhaps someday I'll need to move to inlineParams and queryParams. In the meantime, combining them keeps the API smaller and simpler.

@talamaska
Copy link
Author

But you already have a logic that handles them separately, you are merging them artificially into one collection then you are making additional logic to split them.

@csells
Copy link
Owner

csells commented Oct 5, 2021

I do have logic to join them for the pageBuilder. What logic splits them?

@talamaska
Copy link
Author

@csells
Copy link
Owner

csells commented Oct 5, 2021

I do that to keep the API simple for navigating to named routes -- anything that isn't a positional/indexed parameter is added to the end as a query parameter.

I'm not worried about keeping the implementation simply; I worry about keeping the API and the end-to-end developer experience easy.

@talamaska
Copy link
Author

talamaska commented Oct 5, 2021

I'm not sure what to say here. You obviously already made up your mind and I can't make you think other way. So either I fork it and make it my way, or not. Like i said it's an artificial limitation of how the web urls work. There is no spec that says you can't have same names there. My request was purely based on my experience with other web frameworks and some backend as well. Since everywhere we have them distinguished and you don't have them, I get my answer - you want smaller and simpler. Maybe I'm not getting it how much smaller and simpler is it without handling them in the separate collections in the state.

@csells
Copy link
Owner

csells commented Oct 5, 2021

well, obviously I can't please everyone with any one API but your argument about the spec not requiring the names to be different on the web is a good one... I'll noodle on it. Should it be params/queryParams, inlineParams/queryParams or something else...? I could leave params alone and have all three... hmmm...

@csells
Copy link
Owner

csells commented Oct 5, 2021

I dropped a poll onto Twitter: https://twitter.com/csells/status/1445520767190388738

@talamaska
Copy link
Author

:) you're too nice. Maybe I'm not right. Maybe how it is , is for the best. I'm too spoiled from Angular, web dev who may not understand everything that is mobile and honestly not a huge fan of Flutter for Web. If it comes to naming, I would say params and queryParams is the best. Thanks for your time and patience to explain. Feel free to close that issue.
Best Regards

P.S will check the poll.

@csells
Copy link
Owner

csells commented Oct 7, 2021

hmmm... it looks like the "split it up" proponents are winning the poll. I may need a go_router 2.0 when this is over to make the breaking change clear...

@csells csells added the enhancement New feature or request label Oct 7, 2021
@csells
Copy link
Owner

csells commented Oct 8, 2021

fixed in v2.0.0. @talamaska please verify.

@csells csells closed this as completed Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants