montagejs / collections

This package contains JavaScript implementations of common data structures with idiomatic interfaces.
http://www.collectionsjs.com
Other
2.09k stars 185 forks source link

Collections.js is not loading correctly #158

Open bugs181 opened 8 years ago

bugs181 commented 8 years ago

This was a tricky bug to track down.

Taking the code directly from the examples gives me some very weird results. The issue (in this case) is because the library isn't overriding built-ins.

Let me elaborate.

var Set = require("collections/set")

var object = { x: "hello" }
var set = new Set(["a", object])

console.log( set.add("b") )
// Expected: true
// Result: Set { 'a', { x: 'hello' }, 'b' }

console.log( set.toArray() )
// Expected: ["a",{"x":"hello"},"b"]
// Result: [ 'a', { x: 'hello' }, 'b' ]

console.log( set.add(object) )
// Expected: false
// Result: Set { 'a', { x: 'hello' }, 'b' }

console.log( set.toArray() )
// Expected: ["a",{"x":"hello"},"b"]
// Result: [ 'a', { x: 'hello' }, 'b' ]

In fact, removing the first line gives the exact same output. This leads me to believe that Set is using the built-in operator.

My setup is as follows:

OS: $ sw_vers ProductName: Mac OS X ProductVersion: 10.10.1 BuildVersion: 14B25

Install: $ npm install --save collections

Node version: $ node -v v5.10.0

npm version: $ npm -v 3.8.3

hendrul commented 7 years ago

@bugs181 it seems the documentation is not correct, looking at the code I see this module adds attribute CollectionsSet to built-in Set if this is defined, else it define CollectionsSet as Set globally, so it should be...:

require('collections')
var nameSet = new Set.CollectionsSet(null, function (a, b) {
    return a.name === b.name;
}, function (object) {
    return object.name;
});
nameSet.add({name: "Kris", github: "kriskowal"});
// true
nameSet.add({name: "Stuart", github: "stuk"});
// true
nameSet.get({name: "Kris"});
// {"name":"Kris","github":"kriskowal"}
nameSet.add({name: "Stuart", github: "wrong"});
// false
nameSet.get({name: "Stuart"});
// {"name":"Stuart","github":"stuk"}
kriskowal commented 7 years ago

Does anyone have an opinion of whether the documented behavior or the implemented behavior is better? Anyone else the time to make it so?