search

LedgerHQ / eth-dapp-browser

https://dapp-browser.apps.ledger.com/
5 stars 9 forks source link

"@ledgerhq/react-ui" brakes peer dependencies with "react@18.2.0" #32

Closed rgb2hsl closed 1 year ago

rgb2hsl commented 1 year ago

Steps to reproduce:

  1. Clone https://github.com/LedgerHQ/eth-dapp-browser
  2. Run pnpm install
  3. Get the output: 
    
 ERR_PNPM_PEER_DEP_ISSUES  Unmet peer dependencies

. ├─┬ @ledgerhq/react-ui 0.13.0 │ └─┬ react-chartjs-2 3.3.0 │ └── ✕ unmet peer react@"^16.8.0 || ^17.0.0": found 18.2.0 └─┬ react-select 4.3.1 ├── ✕ unmet peer react@"^16.8.0 || ^17.0.0": found 18.2.0 ├── ✕ unmet peer react-dom@"^16.8.0 || ^17.0.0": found 18.2.0 └─┬ react-input-autosize 3.0.0 └── ✕ unmet peer react@"^16.3.0 || ^17.0.0": found 18.2.0



Seems like `@ledgerhq/react-ui` got updated and brings `react@18.2.0` along, which brakes peer deps of `react-select` and `react-input-autosize`
adrienlacombe commented 1 year ago

what is your pnpm version @rgb2hsl ?

rgb2hsl commented 1 year ago

@adrienlacombe-ledger 7.13.4

but for the record, I've got same result using regular npm

ComradeAERGO commented 1 year ago

@rgb2hsl I opened a PR to remove the warning for react versions above ^18.0 Tested on pnpm 8.6.1

rgb2hsl commented 1 year ago

@ComradeAERGO @adrienlacombe-ledger guys, I've forgot to mention that after the install with current state of repo, if you try to run dev server with 'npm run dev`, there will be a build error due to some modules missing:

npm run dev

> eth-dapp-browser@2.0.0 dev
> next dev

ready - started server on 0.0.0.0:3000, url: http://localhost:3000
info  - Disabled SWC as replacement for Babel because of custom Babel configuration ".babelrc" https://nextjs.org/docs/messages/swc-disabled
info  - Using external babel configuration from /Users/wolfe/projects/p2p/eth-dapp-browser/.babelrc
error - ./node_modules/.pnpm/@ledgerhq+react-ui@0.13.0_ynlbwjpw7oqthgrnao7bjjcd2y/node_modules/@ledgerhq/react-ui/lib/components/asorted/Icon/ProviderIcon.js:1:0
Module not found: Can't resolve '@ledgerhq/icons-ui/react/Providers/index'

Import trace for requested module:
./node_modules/.pnpm/@ledgerhq+react-ui@0.13.0_ynlbwjpw7oqthgrnao7bjjcd2y/node_modules/@ledgerhq/react-ui/lib/components/asorted/Icon/index.js
./node_modules/.pnpm/@ledgerhq+react-ui@0.13.0_ynlbwjpw7oqthgrnao7bjjcd2y/node_modules/@ledgerhq/react-ui/lib/components/asorted/index.js
./node_modules/.pnpm/@ledgerhq+react-ui@0.13.0_ynlbwjpw7oqthgrnao7bjjcd2y/node_modules/@ledgerhq/react-ui/lib/components/index.js
./node_modules/.pnpm/@ledgerhq+react-ui@0.13.0_ynlbwjpw7oqthgrnao7bjjcd2y/node_modules/@ledgerhq/react-ui/lib/index.js
./pages/_app.tsx

https://nextjs.org/docs/messages/module-not-found

Please, make sure that after your changes the server is working fine

@ComradeAERGO regarding the PR — I suppose I'm not the only one who must merge it? But I did my review, anyway :)

ComradeAERGO commented 1 year ago

Thanks for coming back @rgb2hsl. I indeed realized the build fails with react 18.2.

I checked the previous state of the head on 7d400b and saw it didn't fail on build.

From what I see, your warning on peerDeps only pops up if you erase the pnpm-lock file from the repo and generate a new one.

I'll remove this PR.

Please go back to main and try deleting your node_modules and fetching the pnpm-lock file from the repo, and it should just work.

Also worth noting that vercel and I are configured on pnpm 8.6.1

rgb2hsl commented 1 year ago

@ComradeAERGO it works, ty! I've updated pnpm to the latest and it helped. No actions actually needed, it works fine with a fresh clone of the repo.

Issue can be closed.