Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use NAN for Node 0.8->0.11+ compatibility #477

Merged
merged 1 commit into from
Dec 13, 2013
Merged

use NAN for Node 0.8->0.11+ compatibility #477

merged 1 commit into from
Dec 13, 2013

Conversation

rvagg
Copy link
Contributor

@rvagg rvagg commented Dec 6, 2013

Node 0.11 and onwards uses the new V8 which has huge breaking API changes. NAN is designed to make it easy to support V8 and Node across versions 0.8 to 0.11 and onwards without the need for branching macros. NAN is keeping up-to-date as newer versions of Node 0.11 are released and new breaking changes come upstream from V8.

This PR adds NAN support for compiling against the latest Node 0.11; tests all pass. Would be good to get this in npm before the impending 0.12 release.

Let me know if you have any questions.

@@ -11,11 +11,11 @@ test('disconnects', function() {
helper.pg.connect(config, function(err, client, done) {
assert.isNull(err);
client.query("SELECT * FROM NOW()", function(err, result) {
process.nextTick(function() {
setTimeout(function() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.11 currently has a GC problem with certain closures that get GCd before they get invoked, hence the setTimeout()

@rvagg
Copy link
Contributor Author

rvagg commented Dec 6, 2013

/cc #411

@brianc
Copy link
Owner

brianc commented Dec 6, 2013

@rvagg thank you mucho. I've been working at an early stage startup recently (using node-postgres! 👏) with very little time to do a big refactor to support the new version of node & have been kinda scared of getting support out for v0.12.x in time. I cannot thank you enough for going through & doing the hard work on this. I'm going to test this out locally and in our staging environments within the next week or two and get it all merged. One question - do you have a link to docs on NAN? I'd like to dig into it a bit more.

Thanks again!

@rvagg
Copy link
Contributor Author

rvagg commented Dec 6, 2013

@brianc cheers. The docs are all on the readme, we make a point of keeping the docs up to date and comprehensive. Otherwise you can just dig in to nan.h, it's not too scary and there's some extra utilities in there that aren't simply about compatibility.

@brianc
Copy link
Owner

brianc commented Dec 13, 2013

👏

brianc added a commit that referenced this pull request Dec 13, 2013
use NAN for Node 0.8->0.11+ compatibility
@brianc brianc merged commit f58ff73 into brianc:master Dec 13, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants