Skip to content

auto-installing peerDependencies in npm@7 means ipfs-utils installs react-native everywhere. #130

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

Closed
olizilla opened this issue Jun 27, 2021 · 2 comments · Fixed by #131
Closed
Labels
need/triage Needs initial labeling and prioritization

Comments

@olizilla
Copy link
Member

olizilla commented Jun 27, 2021

npm ls react-native 
[email protected] /Users/oli/Code/vasco-santos/ipfs-car
└─┬ [email protected]
  └─┬ [email protected]
    └─┬ [email protected]
      └─┬ [email protected]
        └─┬ [email protected]

a trimmed down look at the node_modules dir after npm install with npm@7 in a project that depends on ipfs-utils now has >100MiB of react-native deps forced on it.

ncdu 1.15.1
--- /Users/oli/Code/vasco-santos/ipfs-car/node_modules ---   
   40.2 MiB [######    ] /react-native
   38.3 MiB [######    ] /jsc-android
   20.1 MiB [###       ] /hermes-engine
    6.7 MiB [#         ] /@react-native-community
    4.1 MiB [          ] /react-devtools-core
    3.1 MiB [          ] /flow-parser

npm@7 started forcing everyone to install the cumulative peerDependencies for any that are not explcitly depended on.

Automatically installing peer dependencies is an exciting new feature introduced in npm 7.
– https://github.blog/2021-02-02-npm-7-is-now-generally-available/#peer-dependencies

the workaround is to npm i --legacy-peer-deps but it seems awkward to expect all consumers to do that.

The culprit (aside from npm@7) is (at least) the dep on react-native-fetch-api which brings in rn as a peerDep:
https://github.com/react-native-community/fetch/blob/8903bb7ee2c673da52f2e84172bef0752f2bedc5/package.json#L61-L62

We might PR that repo to remove the peerDependency, now npm@7 is the current default installed with node@16, but it's not an unreasonable thing for a react-native extention to do... it's exactly what peerDependecies are for. It we can't upstream that change we should fork, as it's not reasonable for something like ipfs-core-types to bring the entire react-native party due to a transitive dep on ipfs-utils

@olizilla olizilla added the need/triage Needs initial labeling and prioritization label Jun 27, 2021
@ipfs ipfs deleted a comment from welcome bot Jun 27, 2021
@olizilla olizilla changed the title auto-installing peerDependecies in npm@7 means ipfs-utils installs react-native everywhere. auto-installing peerDependencies in npm@7 means ipfs-utils installs react-native everywhere. Jun 27, 2021
@acostalima
Copy link
Contributor

acostalima commented Jun 28, 2021

@olizilla I'm the maintainer of react-native-fetch-api. Thanks for bringing up this issue.
I've just released v2.0.0 without the peer dependency. However, it's worth noting that the peer dependency was enforcing the usage of React Native v0.62+, which is the minimum version required for react-native-fetch-api to work. This requirement had already been documented in the package's README, but it should probably be documented somewhere on IPFS' end as well.

@lidel
Copy link
Member

lidel commented Jul 9, 2021

@olizilla got 5min to bump ?

"react-native-fetch-api": "^1.0.2",

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage Needs initial labeling and prioritization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants