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.

Sign up for GitHub

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

Jump to bottom

node-fetch missing in dependency tree #44

Closed
onichandame opened this issue Sep 4, 2021 · 5 comments · Fixed by #78
Closed

node-fetch missing in dependency tree #44

onichandame opened this issue Sep 4, 2021 · 5 comments · Fixed by #78
Labels
released

Comments

@onichandame
Copy link

Problem

This lib requires native-fetch which declares node-fetch as its peerDep. But node-fetch is not a dependency of this lib. Should we expect the app that depends on this lib to install node-fetch or should we have node-fetch in this lib's deps list?
@D4nte
Copy link

D4nte commented Oct 4, 2021

I think node-fetch should be added to this lib's dep list because the problem does exist when you use another lib that brings dns-over-http-resolver in.

For example, my project uses multiaddr@10.0.1 and is facing the same issue:

/home/froyer/src/status-im/js-waku/node_modules/native-fetch/src/index.js
Cannot find module: 'node-fetch'. Make sure this package is installed.

@vasco-santos
Copy link
Owner

vasco-santos commented Oct 18, 2021 •
edited
Loading

Hey folks, thanks for reaching out and apologies for the late answer.
We need to use an environment agnostic fetch implementation here, given we want to support both browser and Node.js.

We can go with https://github.com/web-std/io/tree/main/fetch to make this possible. Any of you would like to submit a PR?

@onichandame
Copy link
Author

The multi-platform compatibility does make this harder to tackle. Axios and isomorphic-fetch are another 2 possible platform-agnostic tools that may help.
As I do not need to use this library anymore, I won't have enough time and testing cases for making a fix. The issue can be kept open to track the progress tho.

@D4nte
Copy link

D4nte commented Oct 26, 2021

It somehow got fixed in my project after some npm clean up 🤔

This was referenced Nov 23, 2022
Closed
Merged
achingbrain pushed a commit that referenced this issue Nov 23, 2022
@bajtos
fix: add undici to satisfy peer dep requirements (#78)
cf7a941 
The module `native-fetch` requires `undici` to be installed as its peer dependency.

Before this change, `pnpm add` was reporting a missing peer dependency.

    dependencies:
    + dns-over-http-resolver 2.1.0

     WARN  Issues with peer dependencies found
    .
    └─┬ dns-over-http-resolver 2.1.0
      └─┬ native-fetch 4.0.2
        └── ✕ missing peer undici@"*"
    Peer dependencies that should be installed:
      undici@"*"

With this change in place, there are no warnings reported.

    dependencies:
    + dns-over-http-resolver 2.1.1

    Progress: resolved 9, reused 9, downloaded 0, added 9, done

Fixes #44
github-actions bot pushed a commit that referenced this issue Nov 23, 2022
@semantic-release-bot
chore(release): 2.1.1 [skip ci]
a692310 
## [2.1.1](v2.1.0...v2.1.1) (2022-11-23)

### Bug Fixes

* add `undici` to satisfy peer dep requirements ([#78](#78)) ([cf7a941](cf7a941)), closes [#44](#44)
@github-actions
Copy link

🎉 This issue has been resolved in version 2.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: add undici to satisfy peer dep requirements bajtos/dns-over-http-resolver
3 participants
@D4nte @vasco-santos @onichandame