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

Update of devDependencies #95

Closed
wants to merge 1 commit into from
Closed

Update of devDependencies #95

wants to merge 1 commit into from

Conversation

ritmas
Copy link
Contributor

@ritmas ritmas commented Jan 30, 2019

Update of devDependencies by preserving backward compatability of older Node / npm versions.

Introduced custom wrapper Utils.Buffer() for handling breaking changes of obsolete new Buffer() constructor, otherwise error will keep occuring like:

(node:XXX) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.

According to nodejs.org guide how to port it, I've chosen Version 3 because:

  • Version 1 drops backward compatability of older Node / npm versions
  • Version 2 would still allow throwing DeprecationWarning warning

@coveralls
Copy link

Coverage Status

Coverage decreased (-19.5%) to 54.958% when pulling 33905dc on ritmas:dev_dep into 1da445d on farhadi:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-19.5%) to 54.958% when pulling 33905dc on ritmas:dev_dep into 1da445d on farhadi:master.

@coveralls
Copy link

coveralls commented Jan 30, 2019

Coverage Status

Coverage increased (+0.6%) to 75.126% when pulling a9a058c on ritmas:dev_dep into 1da445d on farhadi:master.

@farhadi
Copy link
Owner

farhadi commented Jan 31, 2019

@ritmas Thanks for your pull request!
Whats your opinion about dropping support for node versions older than 4?

@ritmas
Copy link
Contributor Author

ritmas commented Jan 31, 2019

Well, it's nice to have support for as wide range of versions as possible. On another hand, it's unclear how would it affect performance.
PS - I'm still working on travis & coveralls fixes

@farhadi
Copy link
Owner

farhadi commented Jan 31, 2019

Probably performance wouldn't be much affected, but to keep dependencies updated we might need to drop support for old node versions that are not supported by dependencies anymore.

@ritmas
Copy link
Contributor Author

ritmas commented Feb 3, 2019

@farhadi probably you're right - eventually we would need to drop support of older/outdated versions, but is v4 the right choice? The most recent one still under maintenance is v6.

@vfcp
Copy link

vfcp commented Mar 27, 2019

I think Version 1 is the best approach. As per my understanding, after Version 1 changes are done, we can still add support for older Node versions by adding safer-buffer polyfill as described in Version 2. safer-buffer is already being added as a dependency of iconv-lite anyways. Once old Node versions support is dropped, it will be as easy as to remove two lines of code and remove safer-buffer dependency. I'll have some free time soon so I'm willing to create PR for this. But, IMHO, just going Version 1 is even better.

Edit: PR #98 was implemented for buffer related changes.

@juliangut
Copy link
Collaborator

juliangut commented May 28, 2019

Buffer creation has been moved to buffer.* keeping compatibility in merged PR #98

Tests pass in node version 7 to 11 as well

We should drop this PR and create a new one for travis file only I guess

@ritmas any thoughts?

@juliangut juliangut modified the milestone: 0.4.0 May 28, 2019
@juliangut juliangut mentioned this pull request Nov 11, 2019
@juliangut
Copy link
Collaborator

closing in favor of #117

@juliangut juliangut closed this Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants