fix: add react-dom as peer dependency

[duarten]

If missing, then strict package managers like pnpm will not link this
react-dom in, causing react-query to fail at runtime cause it can't load
react-dom.

Fixes #3534

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
query	✅ Ready (Inspect)	Visit Preview	Jul 13, 2022 at 3:52PM (UTC)

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4e78c7c:

Sandbox	Source
@tanstack/query-example-react-basic	Configuration
@tanstack/query-example-react-basic-typescript	Configuration

[TkDodo]

thanks. I guess we need react-native as well then, because we have both react-dom and react-native marked as optional in peerDependenciesMeta, right ?

[duarten]

thanks. I guess we need react-native as well then, because we have both react-dom and react-native marked as optional in peerDependenciesMeta, right ?

Yeah, that's right. I can add it to the PR, but I'm not sure what the version selector should be?

[TkDodo]

Yeah, that's right. I can add it to the PR, but I'm not sure what the version selector should be?

yeah me neither. @arnaudbzn might know, or @hirbod ?

[hirbod]

Depends. Expo users need 0.68.2, while RN is available at 0.69.1.
Expo SDK will release SDK 46 soon, which will also be at 0.69.1 (or .2)

[TkDodo]

@hirbod can we just do >0.68.0 ? or is actually any version, so *, enough for us? react-query doesn't really care which version of react-native you use, right?

[hirbod]

It's hard to say. I think folks might end up with the need to add a yarn resolution when different versions being installed.

Potentially, there could still be plenty of 0.64.3 users as well (SDK 44)

I think that * might be the correct way. Irc, @satya164 was tweeting about peer dep issues a while ago. He might know the correct answer.

[TkDodo]

are optional peer dependencies also installed automatically ? I didn't think so, but not sure 🤔

[TkDodo]

see: https://docs.npmjs.com/cli/v7/configuring-npm/package-json#peerdependenciesmeta

Marking a peer dependency as optional ensures npm will not emit a warning if the soy-milk package is not installed on the host. This allows you to integrate and interact with a variety of host packages without requiring all of them to be installed.

[hirbod]

Yeah, that was something similar he was tweeting. Most people did the mistake to add it as devDependency, instead of peer. Adding * should be fine. All expo modules packages I know also add expo as *.

[TkDodo]

thanks. @duarten then let's do react-native: '*' please as a peer dependency

[duarten]

Done!

[satya164]

Keep in mind that adding react-native as a peer dependency means people who don't use react-native and use npm will get react-native installed since npm auto-installs missing peer deps. If they use yarn then they'll get a missing peer dep warning. Same for react-dom and react-native users.

I'd suggest not adding this peer dep. I haven't used pnpm but afaik pnpm users could add some extra config to make this work: pnpm/pnpm#4183 (comment) or maybe there's another way to specify this.

[TkDodo]

sorry @satya164 I just saw your comment right now. I was under the impression that since the dependencies are marked as optional in peerDependenciesMeta, they would not be auto installed with npm. Is that not right?

[satya164]

@TkDodo ah I right, I hadn't checked the full file, yes that should be fine then.

[ingvaldlorentzen]

Seems like npm sadly does auto-install optional peerDepedencies. There is a discussion about it on in the npm/feedback repo. Though there does seem to be some confusion as to what npm is supposed to do, and wether this is a bug or not.

I would suggest react-query remove react-native and react-dom as peerDependencies, i.e. reverse this PR, until this is solved in npm. As installing react-query in a web project now, results in react-native being installed, which adds a lot of potential issues to otherwise simple projects.

It's also worth noting that this is preventing us from setting react and react-dom to a version higher than 18.0.0 as it seems react-native doesn't support e.g. v18.2.0.