Problems getting mobx-react to work with yarn pnp

[marvinhagemeister]

At work we're huge fans of mobx and have been using it without any issues together with react. It's really awesome 👍

We're very excited about the recent work the yarn team did to speed up the package installation process with their recent Plug'n Play-effort, but we're running into an issue with mobx-react because it requires react-dom without declaring it a dependency.

This is the error message thrown by yarn:

ERROR in /Users/marvin.hagemeister/Library/Caches/Yarn/v3/npm-mobx-react-5.2.8-059c7f29254d7cd36e103d79113103b40348d3bf/node_modules/mobx-react/index.module.js
Module not found: Error: Package "[email protected]" (via "/Users/marvin.hagemeister/Library/Caches/Yarn/v3/npm-mobx-react-5.2.8-059c7f29254d7cd36e103d79113103b40348d3bf/node_modules/mobx-react/index.module.js") is trying to require the package "react-dom" (via "react-dom") without it being listed in its dependencies (mobx, react, hoist-non-react-statics, react-lifecycles-compat, mobx-react)
 @ /Users/marvin.hagemeister/Library/Caches/Yarn/v3/npm-mobx-react-5.2.8-059c7f29254d7cd36e103d79113103b40348d3bf/node_modules/mobx-react/index.module.js 3:0-65 639:8-19 641:19-30 1239:11-34 1239:82-105
 @ /Users/marvin.hagemeister/dev/my_project/packages/A/src/App.tsx
 @ /Users/marvin.hagemeister/dev/my_project/packages/A/src/index.tsx
 @ multi /Users/marvin.hagemeister/dev/my_project/.pnp/externals/pnp-adb3ce4a326ccce0073ebdd32468bb0fd11a2bbb/node_modules/webpack-dev-server/client?http://0.0.0.0:3010 (webpack)/hot/dev-server.js /Users/marvin.hagemeister/dev/my_project/packages/A/src/index.tsx

Summoning @arcanis as he does an awesome job at solving these kind of issues 🎉

[arcanis]

Ahah thanks @marvinhagemeister 😊

The incriminated require seems to be done here:

import { unstable_batchedUpdates as rdBatched } from "react-dom"
import { unstable_batchedUpdates as rnBatched } from "react-native"

I think those are provided for optional builtin integrations with react-dom and react-native, is that correct (I'm not entirely sure, because the react-native import doesn't appear in the actual package on the registry)? In such a case, I'd advise adding them to the peerDependencies field from your package.json.

The main problem is that warnings will be emitted if react-dom is missing, but that might be acceptable for now (I'd expect most of mobx-react users to also use react-dom, right? in that sense it might even allow them to catch bugs earlier). This will be fixed in the next version of Yarn thanks to optional peer dependencies.

[mweststrate]

Yes, the react-dom and react-native dependencies are peer but optional (typically you will only use one of them, or possibly even none of them). I don't see a quick way to fix it, although in the future those dependencies might be removed completely

[mweststrate]

PR to add optional peer dependencies and testing if that makes a difference is welcome!

[danielkcz]

Made the PR, but I don't want to dive into the whole PNP thing.

@marvinhagemeister can you perhaps try it out with those optional deps? You can use https://github.com/whitecolor/yalc to make installable build on your local machine.

[danielkcz]

I think it's safe to close this. As of version 6.2.0, we no longer depend on react-dom directly. If some further issues with Yarn PNP appear, please open a new issue.