Skip to content

Use of Buffer.alloc() breaks Node 4.x compatibility #1373

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.

Sign up for GitHub

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

Jump to bottom
Closed
appdlh opened this issue Jul 18, 2017 · 5 comments
Closed

Use of Buffer.alloc() breaks Node 4.x compatibility #1373

appdlh opened this issue Jul 18, 2017 · 5 comments

Comments

@appdlh
Copy link

appdlh commented Jul 18, 2017

The package.json "engines" entry claims compatibility with Node >= 4.0.0, but around pg/lib/connection.js:290:26 there is a call to Buffer.alloc(), an API not introduced until Node 5.0.0. This causes the driver to throw when using Node < 5.x.
@brianc
Copy link
Owner

brianc commented Jul 18, 2017

All the tests are passing on travis for node lts/argon which I believe is node 4.x:

https://travis-ci.org/brianc/node-postgres/jobs/254701876

Should I increase the engines compatibility in package.json to something other than 4.0.0?

@brianc
Copy link
Owner

brianc commented Jul 18, 2017

Looks like I can bump the engines up to >=4.2.x

nodejs/help#28

What do you think?

@appdlh
Copy link
Author

appdlh commented Jul 18, 2017

According to the travis output the tests are running on Node v4.8.4, but that just tells me the tests don't expose this problem... as I said, Buffer.alloc() wasn't introduced until Node 5.0 (according to the API documentation at least).

If the driver is going to continue using 5.x APIs, the minimum engine requirement needs to be at least 5.x; if 4.x support is to be continued, that 5.x API call will need to be addressed.

@charmander
Copy link
Collaborator

They were backported in Node 4.5.0.

@appdlh
Copy link
Author

appdlh commented Jul 18, 2017

Ah, nice; OK, then 4.5.x should be a viable semver spec for the Node "engines" entry.

@charmander charmander mentioned this issue Jul 18, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@brianc @charmander @appdlh