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

url: add sort() to URLSearchParams #10760

Closed
jasnell opened this issue Jan 12, 2017 · 5 comments
Closed

url: add sort() to URLSearchParams #10760

jasnell opened this issue Jan 12, 2017 · 5 comments
Assignees
Labels
whatwg-url Issues and PRs related to the WHATWG URL implementation.

Comments

@jasnell
Copy link
Member

jasnell commented Jan 12, 2017

whatwg/url#26 proposes adding a sort() capability to URLSearchParams. The idea would be to have a simple default lexical sort of keys. If added, we would need to add this to the API

@jasnell jasnell added the whatwg-url Issues and PRs related to the WHATWG URL implementation. label Jan 12, 2017
@targos
Copy link
Member

targos commented Jan 12, 2017

PR: whatwg/url#199

Note: the current proposal requires a stable sorting algorithm.

@TimothyGu
Copy link
Member

whatwg/url#199 has been merged.

Is there an existing stable sort function in the code base? If not, I'll just make a simple insertion sort (or copy the one in V8), which is stable.

@TimothyGu TimothyGu changed the title url: track WHATWG URL issue #26 url: add sort() to URLSearchParams Jan 22, 2017
@TimothyGu TimothyGu self-assigned this Jan 28, 2017
@joyeecheung
Copy link
Member

If we use an array of tuples as the underlying data structure instead of flattening them out, we can just use Array.prototype.sort. Any reason they must be flattened?

@joyeecheung
Copy link
Member

Also I remember there was discussion about moving the parsing bit to C++, for reference, blink does it with a Vector<std::pair<String, String>> and gecko does it with a nsTArray<Param>(essentially a vector with pair of strings too)

@TimothyGu
Copy link
Member

@joyeecheung, we can't use array.sort because it is not guaranteed to be stable, while the WHATWG spec does. In C++, creating new arrays for each tuple seems to be fairly expensive, so I decided on a flattened array instead.

TimothyGu added a commit to TimothyGu/node that referenced this issue Feb 11, 2017
TimothyGu added a commit that referenced this issue Feb 14, 2017
PR-URL: #11098
Fixes: #10760
Ref: whatwg/url#26
Ref: whatwg/url#199
Ref: web-platform-tests/wpt#4531
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
TimothyGu added a commit to TimothyGu/node that referenced this issue Feb 18, 2017
italoacasas pushed a commit to italoacasas/node that referenced this issue Feb 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Development

No branches or pull requests

4 participants