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

Fix package json #4893

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

mayank1513
Copy link

  1. @types/node should be added as a devDependency and not code dependency.
  2. Fix vulnerabilities using npm audit fix

@ricmoo
Copy link
Member

ricmoo commented Dec 9, 2024

That package is necessary as a dependency (opposed to a devDependency) because of the way some platforms (like Angular) depend on some types. I forget the exact details, but I’ll see if I can find the related issue and post it here.

Why is this causing an audit issue?

@ricmoo
Copy link
Member

ricmoo commented Dec 9, 2024

It was related to this issue, which requires the node types for the net package.

@mayank1513
Copy link
Author

Hamm… Seems quite old issue and should have been resolved by now by respective packages.

@ricmoo
Copy link
Member

ricmoo commented Dec 9, 2024

It may be an issue you don’t experience because you either use the browser exports (which removes the IpcProvider) or you use tree-shaking and don’t use the IpcProvider in your code (so the types also get dropped), which is why you won’t encounter this. Or you pull the types in for your own project, which also satisfies the compiler.

But the types reference the net package, so they are required.

In the next major version, the IpcProvider will be moved to an extension package, so this goes away.

But ethers has to deal with a significant number of projects, so backwards compatibility is a high priority. There is a CI for environments you can check out. My guess is if you move that back to dependencies you will see that any of the reference-based environments fail. :(

Can you provide reproduction steps to demonstrate the audit? There are likely other ways to resolve that issue, and I would of course prefer not needing any sort of audit fix. :)

@mayank1513
Copy link
Author

Ah… For NPM audit, it is straight forward. Just checkout to main branch and do npm i. You will get some prompt suggesting 1 high severity vulnerability. Then you can do npm audit (to get the details) or npm audit fix (to auto fix).

@ricmoo
Copy link
Member

ricmoo commented Jan 14, 2025

Maybe your branch is out of date and there was an issue with an older version? Here is a result of a fresh checkout and install:

/tmp/test> git clone [email protected]:ethers-io/ethers.js.git
Cloning into 'ethers.js'...
remote: Enumerating objects: 58082, done.
remote: Counting objects: 100% (372/372), done.
remote: Compressing objects: 100% (176/176), done.
Receiving objects: 100% (58082/58082), 154.51 MiB | 22.81 MiB/s, done.
remote: Total 58082 (delta 291), reused 221 (delta 196), pack-reused 57710 (from 2)
Resolving deltas: 100% (37658/37658), done.
/tmp/test> cd ethers.js
/tmp/test/ethers.js (main)> npm install
npm warn deprecated [email protected]: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
npm warn deprecated [email protected]: Rimraf versions prior to v4 are no longer supported
npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported
npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported
npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported

added 134 packages, and audited 135 packages in 1s

30 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
/tmp/test/ethers.js (main)> npm audit
found 0 vulnerabilities

@ricmoo ricmoo added the discussion Questions, feedback and general information. label Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Questions, feedback and general information.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants