Skip to content
This repository was archived by the owner on Jan 3, 2019. It is now read-only.

URLSearchParams.prototype.sort() is missing #22

Closed
felixfbecker opened this issue May 1, 2017 · 11 comments · Fixed by #23
Closed

URLSearchParams.prototype.sort() is missing #22

felixfbecker opened this issue May 1, 2017 · 11 comments · Fixed by #23

Comments

@felixfbecker
Copy link

The method is defined in the spec here: https://url.spec.whatwg.org/#interface-urlsearchparams

The sort() method, when invoked, must run these steps:

Sort all name-value pairs, if any, by their names. Sorting must be done by comparison of code units. The relative order between name-value pairs with equal names must be preserved.

Run the update steps.

The method is currently missing.

@WebReflection
Copy link
Owner

What's the use case for this?
I've tons of things to do and beside having 100% specs compliant API I'd like to know how important it is for a better estimation, thanks

@felixfbecker
Copy link
Author

I saw it as a blocker for jsdom/whatwg-url adoption, but sounds like that is not going to happen anyway. Mainly wanted to have a proper issue for this open so this is visible for users.

@WebReflection
Copy link
Owner

Also, this is absolutely unfair after all the contribution to specs compliant polyfill I've done for 16 years.

jsdom/whatwg-url#37 (comment)

And after someone insulting me I end up being the one with an attitude?

What the hell is wrong with Donenic !

@WebReflection
Copy link
Owner

sounds like that is not going to happen anyway

Where did you read that?

I just said it's not on my priority list 'cause I've never needed it.

It looks like Domenic will ever accept anything from me but that's an issue for jsdom, not an issue for this poly.

@felixfbecker
Copy link
Author

felixfbecker commented May 1, 2017

sounds like that is not going to happen anyway

Where did you read that?

That's just the impression I got from jsdom/whatwg-url#37 (comment)

I don't want to take any side here or judge anyone, I hope you understand that. I think it would be awesome for the community if we all worked together to fix what needs to be fixed and get this into jsdom/whatwg-url, but I am not in the position to make this happen.

@WebReflection
Copy link
Owner

WebReflection commented May 1, 2017

Until the community tolerates these kind of developers and I get the blame for someone with an attitude toward standards I've advocated and promoted a lifetime, I think we have way more important things to solve as community.

@WebReflection
Copy link
Owner

btw, apparently not a single browser is implementing this .sort method but I'd like to compare my implementation with some native one. Any idea which/when browsers will have it?

@felixfbecker
Copy link
Author

felixfbecker commented May 2, 2017

Node 7 has it implemented:

> require('url').URLSearchParams.prototype
URLSearchParams {
  append: [Function: append],
  delete: [Function: delete],
  get: [Function: get],
  getAll: [Function: getAll],
  has: [Function: has],
  set: [Function: set],
  sort: [Function: sort],
  entries: [Function: entries],
  forEach: [Function: forEach],
  keys: [Function: keys],
  values: [Function: values],
  toString: [Function: toString] }

@WebReflection
Copy link
Owner

in my node 7.2.1 this returns undefined
require('url').URLSearchParams

anyway, I'll try to update the poly with a simple sort implementation first and see how it goes.

At the end, if the goal is to be consistent with the browser cache results (which is the use case I was missing) it looks like it doesn't matter as long as items are sorted consistently every single time so that a simple keys.sort() should do, without going nuts with code points that might require an extra polyfill in.

I will compare results this evening and see what's the outcome.

@felixfbecker
Copy link
Author

I am running Node 7.7.0

@WebReflection
Copy link
Owner

will try that out this evening. Thanks

WebReflection pushed a commit that referenced this issue May 2, 2017
As requested in order to fix #22.

The method is patched for native constructors that haven't implemented it yet.
However, even constructoring via records or sequences might be broken.

In those cases there is still no patch or the whole constructor needs to be substituted.

TODO: decide if it's the case to feature detect upfront and globally replace the class if not standard
      also probably return strings like native (quite some work, not sure if worth it)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants