search

TooTallNate / ref

Turn Buffer instances into "pointers"
http://tootallnate.github.com/ref
453 stars 141 forks source link

Using v8::Object::ForceSet instead of v8::Object:Set #20

Closed sarangsapre closed 9 years ago

sarangsapre commented 9 years ago

Fixing issue https://github.com/TooTallNate/ref/issues/19

ghostoy commented 9 years ago

The build failure is caused by npm on older node.js. @TooTallNate Would you like to merge this?

TooTallNate commented 9 years ago

I don't really get why this is necessary honestly. Shouldn't nan be taking care of this for us? /cc @rvagg @kkoopa

rvagg commented 9 years ago

Off the top of my head (there there isn't very much), this is because of a change to Set for usage where you probably should be using ForceSet anyway. The ForceSet API is consistent so NAN doesn't need to wrap anything. I think this change makes sense and I don't believe there's a role for NAN here.

.. I don't recall the other project where this came up recently but it was the same thing and the same resolution.

kkoopa commented 9 years ago

Yes, I remember something like that too, but not where it showed up. Maybe a closed issue in the nan repo?

On January 10, 2015 1:29:23 AM EET, Rod Vagg notifications@github.com wrote:

Off the top of my head (there there isn't very much), this is because of a change to Set for usage where you probably should be using ForceSet anyway. The ForceSet API is consistent so NAN doesn't need to wrap anything. I think this change makes sense and I don't believe there's a role for NAN here.

.. I don't recall the other project where this came up recently but it was the same thing and the same resolution.

Reply to this email directly or view it on GitHub: https://github.com/TooTallNate/ref/pull/20#issuecomment-69419113

kkoopa commented 9 years ago

It was in node-sqlite3 https://github.com/mapbox/node-sqlite3/pull/369 via NAN https://github.com/rvagg/nan/issues/221

The answer seems to be ForceSet.

TooTallNate commented 9 years ago

Thanks all!