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

Array#sort is underspecified #241

Closed
ljharb opened this Issue Dec 10, 2015 · 4 comments

Comments

Projects
None yet
2 participants
@ljharb
Member

ljharb commented Dec 10, 2015

Reading http://www.ecma-international.org/ecma-262/6.0/#sec-array.prototype.sort, I see that the spec says two conflicting things about the compareFn.

Namely, [1, 2].sort(foo), for a non-undefined, non-function foo, both is implementation-defined, and throws a TypeError (the same language is in ES5).

In Safari and Chrome (and IE < 9), a non-function non-undefined argument is ignored, and the default sort is used.
In Firefox, IE >= 9, and Edge, a non-function non-undefined argument throws a TypeError, as I'd expect.

This seems like something we should specify, and that Safari and Chrome should shift to the non-implementation-defined behavior of throwing for a non-function non-undefined argument. Thoughts?

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb
Member

ljharb commented Dec 10, 2015

(The TypeError in the spec is step 4a in http://www.ecma-international.org/ecma-262/6.0/#sec-sortcompare)

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Dec 10, 2015

Member

I don't think there's a conflict here. For non-function, non-undefined comparefn's, the sort order is implementation defined but the result of the sort function is normatively defined in sec-sortcompare. Safari and Chrome are wrong per spec I believe.

Member

bterlson commented Dec 10, 2015

I don't think there's a conflict here. For non-function, non-undefined comparefn's, the sort order is implementation defined but the result of the sort function is normatively defined in sec-sortcompare. Safari and Chrome are wrong per spec I believe.

@bterlson bterlson closed this Dec 10, 2015

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Dec 10, 2015

Member

Thanks for clarifying! I'll add a patch to es5-shim and a test to the compat-table.

Member

ljharb commented Dec 10, 2015

Thanks for clarifying! I'll add a patch to es5-shim and a test to the compat-table.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Dec 10, 2015

Member

If this isn't covered in test262 it should be :-P /cc @goyakin

Member

bterlson commented Dec 10, 2015

If this isn't covered in test262 it should be :-P /cc @goyakin

ljharb added a commit to es-shims/es5-shim that referenced this issue Dec 11, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment