Permalink
Browse files

1.2.2 fixes spec compliance issues with reduce.

  • Loading branch information...
kriskowal committed Jun 3, 2011
1 parent bef508e commit dcd4da02a652575501fb6c3364356dbe4a9ffdad
Showing with 10 additions and 4 deletions.
  1. +5 −0 CHANGES
  2. +4 −3 es5-shim.js
  3. +1 −1 package.json
View
@@ -1,3 +1,8 @@
1.2.2
- Changed reduce to follow the letter of the spec with regard to having and
owning properties.
- Fixed a bug where RegExps pass as Functions in some engines in reduce.
1.2.1
- Adding few fixes to make jshint happy.
- Fix for issue #12, function expressions can cause scoping issues in IE.
View
@@ -248,7 +248,7 @@ if (!Array.prototype.some) {
if (!Array.prototype.reduce) {
Array.prototype.reduce = function reduce(fun /*, initial*/) {
var len = +this.length;
if (typeof fun !== "function")
if (typeof fun !== "function" || fun instanceof RegExp)

This comment has been minimized.

Show comment
Hide comment
@michaelficarra

michaelficarra Jun 3, 2011

Member

There's no need for a RegExp check here. If typeof X produces "function" then X implements [[Call]].

edit: I know you might say "But, Michael, the spec says 'callbackfn should be a function that takes four arguments.'". And you'd be right. But what is a "function" in that sentence? That's not a technical definition. The only requirement for callbackfn in the technical section is that it implement [[Call]].

edit 2: You might also say "But, Michael, my browser throws a TypeError when I pass a RegExp instance to Array.prototype.reduce". That's because your browser likely didn't fully implement callable RegExps as they are just a short-term experiment.

edit 3: If you absolutely must disallow all [[Call]]able objects that are not Function instances, make a whitelist, not a blacklist. Object.prototype.toString.call(fun) != "[object Function]" should do the trick.

@michaelficarra

michaelficarra Jun 3, 2011

Member

There's no need for a RegExp check here. If typeof X produces "function" then X implements [[Call]].

edit: I know you might say "But, Michael, the spec says 'callbackfn should be a function that takes four arguments.'". And you'd be right. But what is a "function" in that sentence? That's not a technical definition. The only requirement for callbackfn in the technical section is that it implement [[Call]].

edit 2: You might also say "But, Michael, my browser throws a TypeError when I pass a RegExp instance to Array.prototype.reduce". That's because your browser likely didn't fully implement callable RegExps as they are just a short-term experiment.

edit 3: If you absolutely must disallow all [[Call]]able objects that are not Function instances, make a whitelist, not a blacklist. Object.prototype.toString.call(fun) != "[object Function]" should do the trick.

This comment has been minimized.

Show comment
Hide comment
@kriskowal

kriskowal Jun 4, 2011

Member

I need to study this more. This is an area where the letter of the specification isn't sufficient to make the right decision. These are my facts so far.

[].reduce(/./, 1)
V8: throws TypeError
SpiderMonkey allows
JavaScriptCore allows
Trident reduce not implemented

typeof /./
V8 "function"
SpiderMonkey "object"
JavaScriptCore "function"
Trident "object"

/./(".")
Works in SpiderMonkey, V8, JavaScriptCore
Error in Tident

We want the behavior in shimmed engines to match the behavior in the most picky unshimmed engine so our library doesn't hide errors. The most picky is V8. In IE, noticing that typeof fun !== "function" is sufficient to get the TypeError.

So, yeah, I guess the instanceof RegExp check is superfluous. Am I missing anything?

@kriskowal

kriskowal Jun 4, 2011

Member

I need to study this more. This is an area where the letter of the specification isn't sufficient to make the right decision. These are my facts so far.

[].reduce(/./, 1)
V8: throws TypeError
SpiderMonkey allows
JavaScriptCore allows
Trident reduce not implemented

typeof /./
V8 "function"
SpiderMonkey "object"
JavaScriptCore "function"
Trident "object"

/./(".")
Works in SpiderMonkey, V8, JavaScriptCore
Error in Tident

We want the behavior in shimmed engines to match the behavior in the most picky unshimmed engine so our library doesn't hide errors. The most picky is V8. In IE, noticing that typeof fun !== "function" is sufficient to get the TypeError.

So, yeah, I guess the instanceof RegExp check is superfluous. Am I missing anything?

throw new TypeError();
// no value to return if no initial value and an empty array
@@ -280,6 +280,7 @@ if (!Array.prototype.reduce) {
};
}
// ES5 15.4.4.22
// https://developer.mozilla.org/en/Core_JavaScript_1.5_Reference/Objects/Array/reduceRight
if (!Array.prototype.reduceRight) {
@@ -329,7 +330,7 @@ if (!Array.prototype.indexOf) {
if (i < 0)
i += length;
for (; i < length; i++) {
if (!owns(this, i))
if (!(i in this))
continue;
if (value === this[i])
return i;
@@ -349,7 +350,7 @@ if (!Array.prototype.lastIndexOf) {
i += length;
i = Math.min(i, length - 1);
for (; i >= 0; i--) {
if (!owns(this, i))
if (!(i in this))
continue;
if (value === this[i])
return i;
View
@@ -1,7 +1,7 @@
{
"name": "es5-shim",
"description": "ES5 as implementable on previous engines",
"version": "1.2.1",
"version": "1.2.2",
"homepage": "http://github.com/kriskowal/es5-shim/",
"contributors": [
"Kris Kowal <kris@cixar.com> (http://github.com/kriskowal/)",

0 comments on commit dcd4da0

Please sign in to comment.