Skip to content

Commit

Permalink
fix old V8 bug - Weak(Map|Set)#{delete, get, has} should not throw …
Browse files Browse the repository at this point in the history
…error on primitive, close #124
  • Loading branch information
zloirock committed Oct 26, 2015
1 parent dedf7be commit 0f4435b
Show file tree
Hide file tree
Showing 9 changed files with 200 additions and 71 deletions.
8 changes: 5 additions & 3 deletions library/modules/$.collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ var $ = require('./$')
, $def = require('./$.def')
, hide = require('./$.hide')
, forOf = require('./$.for-of')
, strictNew = require('./$.strict-new');
, strictNew = require('./$.strict-new')
, isObject = require('./$.is-object');

module.exports = function(NAME, wrapper, methods, common, IS_MAP, IS_WEAK){
var Base = require('./$.global')[NAME]
Expand All @@ -24,10 +25,11 @@ module.exports = function(NAME, wrapper, methods, common, IS_MAP, IS_WEAK){
if(iterable != undefined)forOf(iterable, IS_MAP, target[ADDER], target);
});
$.each.call('add,clear,delete,forEach,get,has,set,keys,values,entries'.split(','),function(KEY){
var chain = KEY == 'add' || KEY == 'set';
var IS_ADDER = KEY == 'add' || KEY == 'set';
if(KEY in proto && !(IS_WEAK && KEY == 'clear'))hide(C.prototype, KEY, function(a, b){
if(!IS_ADDER && IS_WEAK && !isObject(a))return false;
var result = this._c[KEY](a === 0 ? 0 : a, b);
return chain ? this : result;
return IS_ADDER ? this : result;
});
});
if('size' in proto)$.setDesc(C.prototype, 'size', {
Expand Down
44 changes: 26 additions & 18 deletions modules/$.collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
var global = require('./$.global')
, $def = require('./$.def')
, forOf = require('./$.for-of')
, strictNew = require('./$.strict-new');
, strictNew = require('./$.strict-new')
, isObject = require('./$.is-object')
, fails = require('./$.fails');

module.exports = function(NAME, wrapper, methods, common, IS_MAP, IS_WEAK){
var Base = global[NAME]
Expand All @@ -13,25 +15,33 @@ module.exports = function(NAME, wrapper, methods, common, IS_MAP, IS_WEAK){
var fixMethod = function(KEY){
var fn = proto[KEY];
require('./$.redef')(proto, KEY,
KEY == 'delete' ? function(a){ return fn.call(this, a === 0 ? 0 : a); }
: KEY == 'has' ? function has(a){ return fn.call(this, a === 0 ? 0 : a); }
: KEY == 'get' ? function get(a){ return fn.call(this, a === 0 ? 0 : a); }
: KEY == 'add' ? function add(a){ fn.call(this, a === 0 ? 0 : a); return this; }
: function set(a, b){ fn.call(this, a === 0 ? 0 : a, b); return this; }
KEY == 'delete' ? function(a){
return IS_WEAK && !isObject(a) ? false : fn.call(this, a === 0 ? 0 : a);
} : KEY == 'has' ? function has(a){
return IS_WEAK && !isObject(a) ? false : fn.call(this, a === 0 ? 0 : a);
} : KEY == 'get' ? function get(a){
return IS_WEAK && !isObject(a) ? false : fn.call(this, a === 0 ? 0 : a);
} : KEY == 'add' ? function add(a){ fn.call(this, a === 0 ? 0 : a); return this; }
: function set(a, b){ fn.call(this, a === 0 ? 0 : a, b); return this; }
);
};
if(typeof C != 'function' || !(IS_WEAK || proto.forEach && !require('./$.fails')(function(){
if(typeof C != 'function' || !(IS_WEAK || proto.forEach && !fails(function(){
new C().entries().next();
}))){
// create collection constructor
C = common.getConstructor(wrapper, NAME, IS_MAP, ADDER);
require('./$.mix')(C.prototype, methods);
} else {
var inst = new C
, chain = inst[ADDER](IS_WEAK ? {} : -0, 1)
, buggyZero;
// wrap for init collections from iterable
if(!require('./$.iter-detect')(function(iter){ new C(iter); })){ // eslint-disable-line no-new
var instance = new C
// early implementations not supports chaining
, CHAINING = instance[ADDER](IS_WEAK ? {} : -0, 1)
// V8 ~ Chromium 40- weak-collections throws on primitives, but should return false
, THROWS_ON_PRIMITIVES = fails(function(){ instance.has(1); })
// most early implementations doesn't supports iterables, most modern - not close it correctly
, ACCEPT_ITERABLES = require('./$.iter-detect')(function(iter){ new C(iter); }) // eslint-disable-line no-new
// for early implementations -0 and +0 not the same
, BUGGY_ZERO;
if(!ACCEPT_ITERABLES){
C = wrapper(function(target, iterable){
strictNew(target, C, NAME);
var that = new Base;
Expand All @@ -41,17 +51,15 @@ module.exports = function(NAME, wrapper, methods, common, IS_MAP, IS_WEAK){
C.prototype = proto;
proto.constructor = C;
}
IS_WEAK || inst.forEach(function(val, key){
buggyZero = 1 / key === -Infinity;
IS_WEAK || instance.forEach(function(val, key){
BUGGY_ZERO = 1 / key === -Infinity;
});
// fix converting -0 key to +0
if(buggyZero){
if(THROWS_ON_PRIMITIVES || BUGGY_ZERO){
fixMethod('delete');
fixMethod('has');
IS_MAP && fixMethod('get');
}
// + fix .add & .set for chaining
if(buggyZero || chain !== inst)fixMethod(ADDER);
if(BUGGY_ZERO || CHAINING !== instance)fixMethod(ADDER);
// weak collections should not contains .clear method
if(IS_WEAK && proto.clear)delete proto.clear;
}
Expand Down
8 changes: 5 additions & 3 deletions modules/library/$.collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ var $ = require('./$')
, $def = require('./$.def')
, hide = require('./$.hide')
, forOf = require('./$.for-of')
, strictNew = require('./$.strict-new');
, strictNew = require('./$.strict-new')
, isObject = require('./$.is-object');

module.exports = function(NAME, wrapper, methods, common, IS_MAP, IS_WEAK){
var Base = require('./$.global')[NAME]
Expand All @@ -24,10 +25,11 @@ module.exports = function(NAME, wrapper, methods, common, IS_MAP, IS_WEAK){
if(iterable != undefined)forOf(iterable, IS_MAP, target[ADDER], target);
});
$.each.call('add,clear,delete,forEach,get,has,set,keys,values,entries'.split(','),function(KEY){
var chain = KEY == 'add' || KEY == 'set';
var IS_ADDER = KEY == 'add' || KEY == 'set';
if(KEY in proto && !(IS_WEAK && KEY == 'clear'))hide(C.prototype, KEY, function(a, b){
if(!IS_ADDER && IS_WEAK && !isObject(a))return false;
var result = this._c[KEY](a === 0 ? 0 : a, b);
return chain ? this : result;
return IS_ADDER ? this : result;
});
});
if('size' in proto)$.setDesc(C.prototype, 'size', {
Expand Down
47 changes: 36 additions & 11 deletions tests/library.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 9 additions & 6 deletions tests/library/es6.weak-map.ls
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module \ES6
{freeze} = core.Object
{iterator} = core.Symbol

test 'WeakMap' (assert)->
test 'WeakMap' (assert)!->
assert.isFunction WeakMap
assert.ok \delete of WeakMap::, 'delete in WeakMap.prototype'
assert.ok \get of WeakMap::, 'get in WeakMap.prototype'
Expand Down Expand Up @@ -37,35 +37,38 @@ test 'WeakMap' (assert)->
new WeakMap a
assert.ok done

test 'WeakMap#delete' (assert)->
test 'WeakMap#delete' (assert)!->
assert.isFunction WeakMap::delete
M = new WeakMap [[a = {}, 42], [b = {}, 21]]
assert.ok M.has(a) && M.has(b), 'WeakMap has values before .delete()'
M.delete a
assert.ok !M.has(a) && M.has(b), 'WeakMap hasn`t value after .delete()'
assert.ok (try !M.delete 1), 'return false on primitive'

test 'WeakMap#get' (assert)->
test 'WeakMap#get' (assert)!->
assert.isFunction WeakMap::get
M = new WeakMap!
assert.strictEqual M.get({}), void, 'WeakMap .get() before .set() return undefined'
M.set a = {}, 42
assert.strictEqual M.get(a), 42, 'WeakMap .get() return value'
M.delete a
assert.strictEqual M.get(a), void, 'WeakMap .get() after .delete() return undefined'
assert.ok (try !M.get 1), 'return false on primitive'

test 'WeakMap#has' (assert)->
test 'WeakMap#has' (assert)!->
assert.isFunction WeakMap::has
M = new WeakMap!
assert.ok !M.has({}), 'WeakMap .has() before .set() return false'
M.set a = {}, 42
assert.ok M.has(a), 'WeakMap .has() return true'
M.delete a
assert.ok !M.has(a), 'WeakMap .has() after .delete() return false'
assert.ok (try !M.has 1), 'return false on primitive'

test 'WeakMap#set' (assert)->
test 'WeakMap#set' (assert)!->
assert.isFunction WeakMap::set
assert.ok (w = new WeakMap)set(a = {}, 42) is w, 'chaining'
assert.ok (try new WeakMap!set(42, 42); no; catch => on), 'throws with primitive keys'

test 'WeakMap#@@toStringTag' (assert)->
test 'WeakMap#@@toStringTag' (assert)!->
assert.strictEqual WeakMap::[core.Symbol?toStringTag], \WeakMap, 'WeakMap::@@toStringTag is `WeakMap`'
14 changes: 8 additions & 6 deletions tests/library/es6.weak-set.ls
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module \ES6
{freeze} = core.Object
{iterator} = core.Symbol

test 'WeakSet' (assert)->
test 'WeakSet' (assert)!->
assert.isFunction WeakSet
assert.ok \add of WeakSet::, 'add in WeakSet.prototype'
assert.ok \delete of WeakSet::, 'delete in WeakSet.prototype'
Expand Down Expand Up @@ -34,28 +34,30 @@ test 'WeakSet' (assert)->
new WeakSet a
assert.ok done

test 'WeakSet#add' (assert)->
test 'WeakSet#add' (assert)!->
assert.isFunction WeakSet::add
assert.ok (w = new WeakSet)add({}) is w, 'chaining'
assert.ok (try new WeakSet!add(42); no; catch => on), 'throws with primitive keys'

test 'WeakSet#delete' (assert)->
test 'WeakSet#delete' (assert)!->
assert.isFunction WeakSet::delete
S = new WeakSet!
.add a = {}
.add b = {}
assert.ok S.has(a) && S.has(b), 'WeakSet has values before .delete()'
S.delete a
assert.ok !S.has(a) && S.has(b), 'WeakSet has`nt value after .delete()'
assert.ok (try !S.delete 1), 'return false on primitive'

test 'WeakSet#has' (assert)->
test 'WeakSet#has' (assert)!->
assert.isFunction WeakSet::has
M = new WeakSet!
assert.ok not M.has({}), 'WeakSet has`nt value'
M.add a = {}
assert.ok M.has(a), 'WeakSet has value after .add()'
M.delete a
assert.ok not M.has(a), 'WeakSet has`nt value after .delete()'
assert.ok not M.has(a), 'WeakSet hasn`t value after .delete()'
assert.ok (try !M.has 1), 'return false on primitive'

test 'WeakSet::@@toStringTag' (assert)->
test 'WeakSet::@@toStringTag' (assert)!->
assert.strictEqual WeakSet::[core.Symbol?toStringTag], \WeakSet, 'WeakSet::@@toStringTag is `WeakSet`'
Loading

0 comments on commit 0f4435b

Please sign in to comment.