search

vercel / nft

Node.js dependency tracing utility
https://npmjs.com/@vercel/nft
MIT License
1.26k stars 130 forks source link

Deprecated subdependencies found in @mapbox/node-pre-gyp #421

Open cdwmhcc opened 1 month ago

cdwmhcc commented 1 month ago

Description

When installing dependencies for our project that uses @vercel/nft, we encountered warnings about deprecated subdependencies. These warnings are caused by the @mapbox/node-pre-gyp package, which is a dependency of @vercel/nft.

Details

The following deprecated subdependencies are reported during pnpm install:

Expected Behavior

Dependencies within @vercel/nft should not rely on deprecated packages to ensure better security and compatibility with modern development environments.

Additional Information

styfle commented 1 month ago

There is some related discussion here: https://github.com/vercel/nft/pull/407#issuecomment-2108830341

Would you like to submit a PR to fix it?

benmccann commented 1 week ago

I wonder if we even need to include the dependency at all. I looked at argon2 to see how its used and its just a single function export:

Compare https://npmgraph.js.org/?q=argon2 vs https://npmgraph.js.org/?q=argon2@0.31.1. The new version drops 55 dependencies including @mapbox/node-pre-gyp

benmccann commented 1 week ago

I sent https://github.com/vercel/nft/pull/431 to upgrade argon2. I'm not sure what else is required to address this issue, but I believe this new version of argon2 should hopefully address the blockers and make this issue much easier to solve

styfle commented 1 week ago

Its not related to argon2 at all. I updated my comment from the other issue and I'll repost here.

I looked at how consumers are using it (for example, argon2) and its just a single function export:

const binary = require("@mapbox/node-pre-gyp");
const bindingPath = binary.find(path.resolve(__dirname, "./package.json"));

So perhaps we could implement this find() function easily assuming its just reading the package.json.

Do you want to submit a PR to implement this?

benmccann commented 1 week ago

I see. That makes much more sense to me. Thanks!

It's not exactly clear to me that find is easily replaceable though it would be awesome if someone can figure out how to do that. In the meantime I'll work on cleaning up @mapbox/node-pre-gyp:

benmccann commented 5 days ago

argon2 switched from @mapbox/node-pre-gyp to node-gyp-build here: https://github.com/ranisalt/node-argon2/commit/b47602840a259946039db8526ddd182d1430f634#diff-bc704b883867dea430073059d0e3061c8ac87c037826b06007646f667777916d

The latter library is dramatically lighter. I wonder if we could make the same switch here

benmccann commented 5 days ago

@styfle based on the link from my past comment, do you think something like this might work?

Screenshot from 2024-06-30 20-37-33

It looks like argon2 was able to do something similar, but when I create a dummy find method that returns null I don't get any extra failing tests (I have 6 that fail for me both on main and with a clearly broken implementation), so I'm not overly confident in the fix. I'm wholly unfamiliar with nft so don't know if I can put together and test a fix, but I hope that discovering the argon2 implementation uses node-gyp-build to do the same as @mapbox/node-pre-gyp's find will be enough to let you address this.

styfle commented 4 days ago

nft supports both node-gyp-build and @mapbox/node-pre-gyp because it needs to work on any version of any npm package