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

Avoid rearrange query parameters #946

Merged
merged 4 commits into from
Nov 28, 2016
Merged

Avoid rearrange query parameters #946

merged 4 commits into from
Nov 28, 2016

Conversation

miccycn
Copy link
Contributor

@miccycn miccycn commented Nov 28, 2016

@fnlctrl
Copy link
Member

fnlctrl commented Nov 28, 2016

/ping @yyx990803

@LinusBorg
Copy link
Member

LinusBorg commented Nov 28, 2016

The order of keys returned by Object.keys() is not reliable:

The Object.keys() method returns an array of a given object's own enumerable properties, in the same order as that provided by a for...in loop

A for...in loop iterates over the properties of an object in an arbitrary order (see the delete operator for more on why one cannot depend on the seeming orderliness of iteration, at least in a cross-browser setting).

so I don't think that simply removing the .sort() could produce a reliable solution to the problem of #926 and will instead break "randomly".

@fnlctrl
Copy link
Member

fnlctrl commented Nov 28, 2016

@LinusBorg It's been discussed in #926, sorry it's in Chinese since OP switched to it in the middle of it. Basically the reasons are:

  1. Developer should never rely on the order of query parameters. So removing .sort() shouldn't be a breaking change for those who don't. If they do rely on query order, it should serve as a warning to let them switch to dynamic segments.
  2. OP agreed that this change is unreliable, but it works well on his target browsers so it's good enough for him. Since only he cares about it (I obviously don't), and as said in 1). people should never rely on query order, it's a good compromise to avoid complicating current code just to preserve query params order.
  3. Evan replied him in weibo (Chinese version of twitter) and instructed him to remove the sort, so I think it 's fine.

@miccycn
Copy link
Contributor Author

miccycn commented Nov 28, 2016

Yes, it is not reliable.

It is not the best way to solve it, but I think Vue-Router is not responsible for sorting the query string by its keys.
But now it cause some trouble.
If Vue-Router dose not do that, once there are problems in this case, it's browser`s responsibility, not Vue-Router.

@LinusBorg
Copy link
Member

Well, that is one way to see it, but another way would be that because the browser does not guarantee order, vue-router does, with .sort - but I can also understand your problem with #926

Both scenarios are not optimal.

@miccycn
Copy link
Contributor Author

miccycn commented Nov 28, 2016

@LinusBorg
Actually, I am more curious about what it is necessary to do so, coz other functions seems do not need an ordered query.

@yyx990803 yyx990803 merged commit d4de8c8 into vuejs:dev Nov 28, 2016
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

Successfully merging this pull request may close these issues.

4 participants