Upgrade NAN for Node 4.0.0

[brianmcd]

No description provided.

[brianmcd]

@kkoopa - is this something you can help with? If not, is there an upgrade guide floating around, or should I work my way through the NAN changelog?

[kkoopa]

Is it still going to be maintained? Would it not be better to write something on the javascript level that uses the existing code for versions less than 0.12, and the revised vm module of Node for everything later.

Upgrading to NAN 2 wouldn't be hard, I can do it in ten minutes, but is it worth it?

[mgol]

The problem is there's no way to write in package.json "install this module only in Node<4.0.0". You can declare it as an optional dependency but it will then try to get compiled in newer Nodes & fail; everything should work afterwards but the installation shouldn't have to try to compile things that are not needed, burning CPU cycles & printing confusing error-not-error messages.

[kkoopa]

Ah, I had not thought about that, but it is easily avoidable. Hide everything behind an ifdef and make it compile nothing.

[ChALkeR]

Adding to the list: nodejs/node#2798.

[mgol]

FWIW, @rvagg has a PR in progress: #181.

[brianmcd]

That's a cool thought, @kkoopa. My current plan is to test/merge #181 to get builds working ASAP, then look into doing the JS implementation for Node 4 and up.

[kkoopa]

It might make sense to use the built-in vm module already from 0.12 up, since @domenic based the rewrite on Contextify. I would think many bugs that were in Contextify have been fixed in the vm module in that process. Essentially, the C++ code of Contextify is only necessary for Node 0.10 and older.

[thelinuxlich]

+1

[soenkekluth]

+1 actually one of our projects completely won't work on node 4.x just because of this...

[rvagg]

sorry, I'm blocked on one failure in #181 and don't have time to figure it out right now, happy for someone else to take my code if they can work it out

[KingScooty]

+1 It's affects a lot of packages i need since upgrading to node 4.x. It appears Browser Sync and a few of its plugins require it.

[raiskila]

+1 Breaks lots of things

[base698]

Using node 4.1.0

$ npm install contextify

contextify@0.1.14 install /Users/workspaces/soundselect/node_modules/contextify
node-gyp rebuild

CXX(target) Release/obj.target/contextify/src/contextify.o
In file included from ../src/contextify.cc:3:
../node_modules/nan/nan.h:261:25: error: redefinition of '_NanEnsureLocal'
NAN_INLINE v8::Local _NanEnsureLocal(v8::Local val) {
^
../node_modules/nan/nan.h:256:25: note: previous definition is here
NAN_INLINE v8::Local _NanEnsureLocal(v8::Handle val) {
^
../node_modules/nan/nan.h:661:13: error: no member named 'smalloc' in namespace 'node'
, node::smalloc::FreeCallback callback
~~~~~~^
../node_modules/nan/nan.h:672:12: error: no matching function for call to 'New'
return node::Buffer::New(v8::Isolate::GetCurrent(), data, size);
^~~~~~~~~~~~~~~~~
/Users/justinthomas/.node-gyp/4.1.0/include/node/node_buffer.h:31:40: note: candidate function not viable: no known conversion from
'uint32_t' (aka 'unsigned int') to 'enum encoding' for 3rd argument
NODE_EXTERN v8::MaybeLocalv8::Object New(v8::Isolate* isolate,

[rvagg]

If you're desperate then you can npm i 'contextify@rvagg/contextify#nan2' but see #181 for the context (har har) on that. It should be fine on 0.12+ but the fact that there's a weird hold-up on 0.10 suggests that all may not be quite right.

[mgol]

If finishing up the nan2 branch proves difficult to finish fast maybe @kkoopa's solution from #180 (comment) would work?

[base698]

I'm not using contextify directly--its used by several other modules like
jsdom. I could fork all of those and reference the pull requested version
but that seems non optimal.
On Sep 21, 2015 17:59, "Michał Gołębiowski" notifications@github.com
wrote:

If finishing up the nan2 branch proves difficult to finish fast maybe the
solution from #180 (comment)
#180 (comment)
would work?

—
Reply to this email directly or view it on GitHub
#180 (comment)
.

[slavaGanzin]

Just a comment to be in touch

Thanks for your work guys 👍

[tonycurwen]

So clearly not a fix, but if you're deploying to Heroku just force Node 0.12.7 in the package.json file for your project until the 4.x support is resolved. Worked for me though I don't have any Node 4.0/4.1 dependencies at this time.

    "engines": {
        "node": "0.12.7",
        "npm": "2.11.3"
    },

I mention Heroku as this defect was blocking new deploys for me due to a recent Heroku Node upgrade.

[idoby]

My 4.0.0 port #190 doesn't seem to compile on @brianmcd's CI server, but it should work if you have a C++11 compiler (required by the new NAN).

[skeller88]

+1

[abhas9]

+1 for node 4.x support.

[rvagg]

I'm really looking forward to knowing what we'll be able to buy with all these +1's

[idoby]

[caljrimmer]

+1

[aaronryden]

👍

[y9v]

+1

[jfraboni]

+1

[rpeterson]

+1

[reality]

+1

[why520crazy]

+111111111111111111111111111111111111111

[sattaman]

+1

[SylvainCorlay]

+1

[kerawits]

+1

[afcastano]

+1

[DKnodel]

+1

[josephhealy]

+1

[pyephyomaung]

+1

[poshaughnessy]

This fixed it for me for now - thanks @sirbrillig: sirbrillig/jsdom@b30bc08

[brianmcd]

Needs a few more +1s

[brianmcd]

(Fixed in 0.1.15)

[mikemaccana]

+thanks