Making tldr compatible with node 10

[vladimyr]

Description

Swapping unzip2 with node-unzip-2 fork (uses graceful-fs@4) in order to avoid node 10 core dump.
Source: nodejs/node#20285 (comment)

Checklist

Please review this checklist before submitting a pull request.

• Code compiles correctly
• Created tests, if possible
• All tests passing (npm run test:all)
• Extended the README / documentation, if necessary

[vladimyr]

@agnivade There were 2 different things that made node 10 fail:

1. old version of unzip2 package that used problematic graceful-fs@3 under the hood causing core dump
2. caching node_modules folder on subsequent installs on travis

Second is problematic because webworker-threads needs to be rebuilt when you go from node 9 to node 10. So we need to either stop caching node_modules between different environments/installs or make sure that we call npm rebuild before we run tests. First option makes more sense to me but if you want it other way around let me know 😉

[agnivade]

caching node_modules folder on subsequent installs on travis

Ah so that was the issue ! I was investigating but could not figure out the webworker failures. I have a different approach without changing the unzip2 dependency here at #215, which I believe is slightly more safe than changing the dependency altogether.

Let me know what you think.

[vladimyr]

Hm, I also bumped natural version but it doesn't really change anything because it did not cause failures in the first place... Am I missing something? 🤔

Regarding dependency change, it is actually exactly same codebase just with updated deps:

1. [email protected] published 4 yrs ago using graceful-fs@3 through [email protected]
2. [email protected] published few months ago using graceful-fs@4 through [email protected]

so technically it is a depedency change but what it does it just updates dependencies of unzip2 so we can get rid of problematic graceful-fs@3 that caused node 10 to core dump.

[vladimyr]

Now I'm confused 💫
Checked your branch, and it really does not have graceful-fs@3 inside package-lock.json which is superweird due to following:

tldr-node-client/package-lock.json

Lines 1897 to 1903 in 240f957

	"unzip2": {
	"version": "0.2.5",
	"resolved": "https://registry.npmjs.org/unzip2/-/unzip2-0.2.5.tgz",
	"integrity": "sha1-TveleaeMFcUfVQ9qBT2xlBSciZI=",
	"requires": {
	"binary": "0.3.0",
	"fstream": "0.1.31",

tldr-node-client/package-lock.json

Lines 743 to 748 in 240f957

	"fstream": {
	"version": "0.1.31",
	"resolved": "https://registry.npmjs.org/fstream/-/fstream-0.1.31.tgz",
	"integrity": "sha1-czfwWPu7vvqMn1YaKMqwhJICyYg=",
	"requires": {
	"graceful-fs": "4.1.11",

But in the same time, package.json of [email protected] says that it requires "graceful-fs": "~3.0.2"...

Did you change package-lock.json manually ❓ 😉
Because if you did that would work only on Travis. CI environment gets code through git and thus fetches package-lock.json which isn't normally published on npm. Regular users won't get it and they'll still get graceful-fs@3 so we'll end up in volkswagen state (green test + broken app) 😁

[agnivade]

Did you change package-lock.json manually

Yes, I did 😆

package-lock.json which isn't normally published on npm.

Oh shucks .. didn't think of that. Ok, let us go with node-unzip2.