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

Feedback please: Node v4 as minimum required runtime #610

Closed
lovell opened this issue Oct 21, 2016 · 15 comments
Closed

Feedback please: Node v4 as minimum required runtime #610

lovell opened this issue Oct 21, 2016 · 15 comments

Comments

@lovell
Copy link
Owner

lovell commented Oct 21, 2016

Maintenance for Node v0.10 ends in 10 days' time, which means no more security fixes for things like OpenSSL bugs.

Maintenance for Node v0.12 is up at the end of this year, just over two months' time.

https://github.com/nodejs/LTS#lts-schedule

The usual Node version laggards seem to be ready for v4:

  • AWS Lambda v4.3.2
  • Meteor v4.4.7

I'd like to drop support for versions of Node prior to v4 in the next "major" release, v0.17.0.

Are there any objections or reasons why we shouldn't do this? Feedback very much welcome here.

@lovell lovell added this to the v0.17.0 milestone Oct 21, 2016
@sachin-walia
Copy link

@lovell - is there a constraint on what maximum version of Node I can use with sharp. At present for my application I am using Node 6.9 and wondering if sharp will have any compatibility issue?

@lovell
Copy link
Owner Author

lovell commented Oct 22, 2016

@sachinwalia2k8 With each major Node version there are often breaking API changes with the version of V8. Sometimes these require updates to nan upon which this module depends. This change might then require a version bump and release of this module, but it depends.

There are CI environments for Linux, OS X and Windows, the config for which lists the versions being tested against. This includes the latest v6.x.x release.

The changelog contains details about support for a new major version of Node only when a change is required for that version. It will also contain details of dropping support for older versions when we do so.

@sachin-walia
Copy link

@lovell - Thanks for the info. For what it's worth I tried installing it against Node 6.9.1 and I am getting an error. I am attaching the npm-debug.log if you want to take a look at it and suggest what would be the approach to resolve it. Thanks a lot for your help.
npm-debug.txt

@lovell
Copy link
Owner Author

lovell commented Oct 22, 2016

@sachinwalia2k8 Probably #602 (comment), new issue if not please.

@sachin-walia
Copy link

@lovell - Thanks. Existing install of libvips 8.4+ via homebrew seems to be an issue.

@wtgtybhertgeghgtwtg
Copy link

This would mean you could drop the bluebird dependency, right?

@lovell
Copy link
Owner Author

lovell commented Oct 22, 2016

@wtgtybhertgeghgtwtg Good question. I believe the use of native Promises may incur a slight performance hit - see some relevant discussion at #55 (comment) - but we can benchmark this.

@wtgtybhertgeghgtwtg
Copy link

It probably will, but it's a 599KB dependency duplicating natively available stuff. So you could see it as a choice between absolute performance or a smaller dependency graph.

@lovell
Copy link
Owner Author

lovell commented Oct 23, 2016

For those interested in reducing the number of bytes downloaded and/or stored on disk, a move to Node v4 as a minimum will allow a move from request to a module more focussed on the simple GET-a-file use case (e.g. no cookies) like the got + caw combo.

@wtgtybhertgeghgtwtg
Copy link

Unless #593 makes it. And it's not just about disk size. More dependencies means more places it can break.

@lovell
Copy link
Owner Author

lovell commented Oct 23, 2016

A slightly modified version of the existing sharp benchmark test run locally with Node v6.9.1 (V8 v5.1) suggests V8's Promise implementation has a ~-0.5% impact on performance compared with the Bluebird implementation.

http://v8project.blogspot.co.uk/2016/07/v8-release-53.html suggests there is at least a 20% performance improvement to native Promise performance in V8 v5.3 and this will be in the forthcoming Node v7 (V8 v5.4).

@lovell
Copy link
Owner Author

lovell commented Oct 25, 2016

I've managed to reduce the total number of production (sub-)dependencies from 93 to 50, with an associated disk usage reduction of 5.2MB.

@sachin-walia
Copy link

@lovell - NodeJS 7 has been released today and I can try out if sharp has any compatibility issues.

@sachin-walia
Copy link

I have been testing sharp with Node 7.0 and haven't encountered any issues so far.

@lovell
Copy link
Owner Author

lovell commented Dec 11, 2016

v0.17.0 now available with a Node v4 minimum runtime dependency. Thanks for all the +ve feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants