fivdi / epoll

A low-level Node.js binding for the Linux epoll API
MIT License
84 stars 13 forks source link

Add node-gyp to package dependencies #40

Closed dawn-minion closed 3 years ago

dawn-minion commented 4 years ago

Can we add node-gyp to the package dependencies? Currently it's expected to be installed, but not listed as one of the things this package needs. This means Yarn PNP can't install a project that depends on this, and the user has to already have node-gyp in the path.

fivdi commented 3 years ago

Can we add node-gyp to the package dependencies? Currently it's expected to be installed, but not listed as one of the things this package needs.

This package doesn't really need node-gyp. npm or yarn need it to install this package.

I'm not familiar with Yarn Plug’n’Play but after taking a look at the Getting Started page and giving it a try I haven't encountered any issues and don't understand why node-gyp needs to be added as a dependency.

Here is what I tried:

pi@raspberrypi:~ $ node --version
v14.11.0
pi@raspberrypi:~ $ yarn --version
1.22.5
pi@raspberrypi:~ $ mkdir test
pi@raspberrypi:~ $ cd test/
pi@raspberrypi:~/test $ yarn init
yarn init v1.22.5
question name (test): 
question version (1.0.0): 
question description: 
question entry point (index.js): 
question repository url: 
question author: 
question license (MIT): 
question private: 
success Saved package.json
Done in 2.21s.
pi@raspberrypi:~/test $ yarn add epoll
yarn add v1.22.5
info No lockfile found.
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
success Saved 4 new dependencies.
info Direct dependencies
└─ epoll@4.0.0
info All dependencies
├─ bindings@1.5.0
├─ epoll@4.0.0
├─ file-uri-to-path@1.0.0
└─ nan@2.14.1
Done in 5.29s.
pi@raspberrypi:~/test $ yarn --pnp
yarn install v1.22.5
info Plug'n'Play support has been greatly improved on the Yarn v2 development branch.
info Please give it a try and tell us what you think! - https://next.yarnpkg.com/getting-started/install
[1/5] Resolving packages...
[2/5] Fetching packages...
[3/5] Linking dependencies...
[5/5] Building fresh packages...
Done in 6.36s.
pi@raspberrypi:~/test $ 

I also did this:

pi@raspberrypi:~/test $ ls -la
total 60
drwxr-xr-x  4 pi pi  4096 Sep 25 04:30 .
drwxr-xr-x 13 pi pi  4096 Sep 25 04:29 ..
drwxr-xr-x  2 pi pi  4096 Sep 25 04:30 node_modules
-rw-r--r--  1 pi pi   174 Sep 25 04:30 package.json
drwxr-xr-x  3 pi pi  4096 Sep 25 04:30 .pnp
-rwxr-xr-x  1 pi pi 33827 Sep 25 04:30 .pnp.js
-rw-r--r--  1 pi pi  1216 Sep 25 04:30 yarn.lock
pi@raspberrypi:~/test $ rm -rf node_modules/
pi@raspberrypi:~/test $ rm -rf .pnp
pi@raspberrypi:~/test $ rm -rf .pnp.js 
pi@raspberrypi:~/test $ ls -la
total 16
drwxr-xr-x  2 pi pi 4096 Sep 25 04:31 .
drwxr-xr-x 13 pi pi 4096 Sep 25 04:29 ..
-rw-r--r--  1 pi pi  174 Sep 25 04:30 package.json
-rw-r--r--  1 pi pi 1216 Sep 25 04:30 yarn.lock
pi@raspberrypi:~/test $ yarn install
yarn install v1.22.5
info Plug'n'Play support has been greatly improved on the Yarn v2 development branch.
info Please give it a try and tell us what you think! - https://next.yarnpkg.com/getting-started/install
[1/5] Resolving packages...
[2/5] Fetching packages...
[3/5] Linking dependencies...
[5/5] Building fresh packages...
Done in 7.25s.
pi@raspberrypi:~/test $ ls -la
total 56
drwxr-xr-x  3 pi pi  4096 Sep 25 04:31 .
drwxr-xr-x 13 pi pi  4096 Sep 25 04:29 ..
-rw-r--r--  1 pi pi   174 Sep 25 04:30 package.json
drwxr-xr-x  3 pi pi  4096 Sep 25 04:31 .pnp
-rwxr-xr-x  1 pi pi 33827 Sep 25 04:31 .pnp.js
-rw-r--r--  1 pi pi  1216 Sep 25 04:30 yarn.lock
pi@raspberrypi:~/test $ 

It looks like everything works to me. Can you describe how to reproduce the problem please?

dawn-minion commented 3 years ago

Hm, it seems I can't reproduce this in the newer Yarn version anymore, apologies - tried it in a fresh Docker image too. I'm still not quite sure how it knows to bring in node-gyp, but hey if it works :smile: Will close then.

fivdi commented 3 years ago

That's good news. Thank you for providing the feedback.