tryretool / react-retool

MIT License
39 stars 14 forks source link

Update dependencies, remove package-lock #44

Closed BenjamynBaker closed 1 year ago

BenjamynBaker commented 1 year ago
shawntax-retool commented 1 year ago

Is there a reason why peer dependencies for react and react-dom need to be kept at 17?

Folks who's projects use React 18 (e.g using the latest CRA) and want to use npm 7 or later, they will run into

Could not resolve dependency: peer react@"^17.0.2" from react-retool@1.1.2
react-retool@"1.1.2" from the root project
Conflicting peer dependency: react@17.0.2

when running npm install, and will have to add the --legacy-peer-deps flag explicitly when adding/removing deps. So peerDependencies need to be set to 18 if we want to support the latest version of React.

Also, upgrading devDependencies to React 18 without making the changes described in the docs here, means we're still effectively on React 17, as described by this warning during local dev:

Warning: ReactDOM.render is no longer supported in React 18. Use createRoot instead. Until you switch to the new API, your app will behave as if it's running React 17. Learn more: https://reactjs.org/link/switch-to-createroot
kentwalters commented 1 year ago

Good questions.

This should really work with either React 17 or 18. We could do something like this: https://mariosfakiolas.com/blog/install-multiple-major-versions-of-a-node-module-with-npm/

But in lieu of something fancy, I'm on board with defaulting to the same major version that CRA uses.

Kent Walters San Francisco

On Thu, Dec 15, 2022 at 10:42 AM, Shawn Taxerman < @.*** > wrote:

Is there a reason why peer dependencies for react and react-dom need to be kept at 17?

Folks who's projects use React 18 (e.g using the latest CRA) and want to use npm 7 or later, they will run into

Could not resolve dependency: peer react@"^17.0.2" from @. react-retool@"1.1.2" from the root project Conflicting peer dependency: @.

when running npm install , and will have to add the --legacy-peer-deps flag explicitly when adding/removing deps. So peerDependencies need to be set to 18 if we want to support the latest version of React.

Also, upgrading devDependencies to React 18 without making the changes described in the docs here ( https://reactjs.org/blog/2022/03/08/react-18-upgrade-guide.html ) , means we're still effectively on React 17, as described by this warning during local dev:

Warning: ReactDOM.render is no longer supported in React 18. Use createRoot instead. Until you switch to the new API, your app will behave as if it's running React 17. Learn more: https:/ / reactjs. org/ link/ switch-to-createroot ( https://reactjs.org/link/switch-to-createroot )

— Reply to this email directly, view it on GitHub ( https://github.com/tryretool/react-retool/pull/44#issuecomment-1353545528 ) , or unsubscribe ( https://github.com/notifications/unsubscribe-auth/ADMX2P5R7GZRR56IQCAEAJLWNNRAJANCNFSM6AAAAAAS7KJBBU ). You are receiving this because your review was requested. Message ID: <tryretool/react-retool/pull/44/c1353545528 @ github. com>

BenjamynBaker commented 1 year ago

Is there a reason why peer dependencies for react and react-dom need to be kept at 17? I think we should support any React version > v17. I updated it to do something like this. I thought ^ would = at least that version but apparently not.

means we're still effectively on React 17 I am OK with functionally staying on v17 in our development example for now. The main motivation here was to update the recommended packages, even if that doesn't mean a full cutover to React v18 in our dev example. I think we can follow up on that in a new PR.

LMK