4.0.0-next.77 sample app yields "Module not found: Your application tried to access react-refresh" when started with Yarn 2

[volodymyr-kushnir]

Describe the bug

[email protected] yields "Module not found: Your application tried to access react-refresh, but it isn't declared in your dependencies; this makes the require call ambiguous and unsound." after running yarn start with Yarn 2. Installing react-refresh as dependency solves the problem, tho.

Environment

Environment Info:

  current version of create-react-app: 4.0.0-next.77
  running from /Users/v.kushnir/.npm/_npx/57508/lib/node_modules/create-react-app

  System:
    OS: macOS 10.15.6
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
  Binaries:
    Node: 12.16.3 - /usr/local/bin/node
    Yarn: 2.1.1 - /usr/local/bin/yarn
    npm: 6.14.4 - /usr/local/bin/npm
  Browsers:
    Chrome: 84.0.4147.105
    Edge: Not Found
    Firefox: Not Found
    Safari: 13.1.2
  npmPackages:
    react: Not Found
    react-dom: Not Found
    react-scripts: Not Found
  npmGlobalPackages:
    create-react-app: Not Found

Steps to reproduce

1. npx create-react-app@next --scripts-version=@next --template=cra-template@next my-js-app
2. cd my-js-app
3. yarn set version berry
4. yarn set version latest
5. yarn install
6. yarn start

Expected behavior

App is served without problems.

Actual behavior

App throws an error.

[volodymyr-kushnir]

So if I understand correctly, react-refresh-webpack-plugin basically injects require("react-refresh/runtime"); into every module and then, when webpack asks for it, Yarn 2 thinks that the call is actually coming from my own modules and, since I haven't listed react-refresh as a dependency, it complains.

It seems like we can't fix it other than adding a note to the README.md asking people, who use Yarn 2, to also install react-refresh. Or we can make it a peer dependency and ask everybody to install it, and that would probably be more semantically correct, I'm not sure.

[eddiemonge]

So crazy story. Do npm install and then try starting it again. To me, that seems like a bug in Yarn2

[volodymyr-kushnir]

@eddiemonge it works with npm and it works with Yarn 1.22.4 as well, because both of them use node_modules. Yarn 2 on the other hand keeps packages in .yarn/cache and complains if packages try to require modules they didn't list as their dependencies, because, even though it has react-refresh hoisted to the top, modules must explicitly list versions of dependencies they need.
What I'm saying is that create-react-app 4 won't work with PnP in Yarn 2 unless react-refresh is also listed as a dependency 🤷🏼‍♂️. All other setups would work fine. I'm just curious if anybody has an elegant solution for this specific use case.

[pmmmwh]

Maintainer of the fast refresh plugin here.

To me, that seems like a bug in Yarn2

Technically speaking, it is always unsound to import something that you haven't listed in your dependencies. Through hoisting it might work (for example in CRA v3 @babel/core), but importing stuff that was never listed means the installation chain is prone to breakage.

So if I understand correctly, react-refresh-webpack-plugin basically injects require("react-refresh/runtime"); into every module and then, when webpack asks for it, Yarn 2 thinks that the call is actually coming from my own modules and, since I haven't listed react-refresh as a dependency, it complains.

Yes, that is the case. We had to do that (kinda like how @babel/runtime works) because there is no other way for us to inject a globally available dependency throughout the bundle without leaking implementation into the actual global scope.

What I'm saying is that create-react-app 4 won't work with PnP in Yarn 2 unless react-refresh is also listed as a dependency

To me I think the best resolution to this problem would be to list react-refresh as an installed dependency by default in the templates, and also a peerDependency of react-scripts. While this kinda breaks the "single-dependency" promise, given that we're already shipping RTL as a default in the templates, I don't see how this is problematic.

I can PR this change (or actually, anyone can do it) - but I'll wait for the maintainers' point of view on this before progressing.

[stale]

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

[pmmmwh]

Bump - I think I'll proceed on making a PR to push further discussions.

[stale]

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

[pmmmwh]

Bump - #9671

[jkolyer]

Hitting this error now. Does react-refresh go into devDependencies, or dependencies?

[derekschinke]

When I add react-refresh to dependencies, it still doesn't work. I get a new error, Failed to load config "react-app" to extend from.

Edit: Possible solution! run yarn add react-refresh eslint-config-react-app

[volodymyr-kushnir]

Oh yeah, same.
4.0.0-next.77 and 4.0.0-next.98 would work, but stable 4.0.0 won't, unless you remove eslintConfig key from your package.json. I haven't figured it out yet, but it seems like react-scripts instructs ESLint to extend the react-app config and it works as long as you don't extend the ESLint config with your own rules, because then react-app is nowhere to be found by ESLint, since normally it would look into node_modules which isn't here, and it can't rely on Yarn, because this config isn't listed in the dependencies either 🤷🏼‍♂️.
For now I assume it's either going back to Yarn Classic or not extending the default CRA ESLint config ⚖️. Having Yarn 2 is nice. Having a customized linter is also nice. Bummer 😔.

[merceyz]

For now I assume it's either going back to Yarn Classic

@volodymyr-kushnir Or you know, enabling the node-modules linker in V2 using yarn config set nodeLinker node-modules or simply adding the missing dependencies using yarn add react-refresh eslint-config-react-app

So crazy story. Do npm install and then try starting it again. To me, that seems like a bug in Yarn2

@eddiemonge That's not a solution and it's not an issue in Yarn 2's PnP mode, they are simply undeclared dependencies

To me I think the best resolution to this problem would be to list react-refresh as an installed dependency by default in the templates, and also a peerDependency of react-scripts. While this kinda breaks the "single-dependency" promise, given that we're already shipping RTL as a default in the templates, I don't see how this is problematic.

@pmmmwh It would be better for the plugin to resolve the import before injecting it, that way it works no matter what, PR at pmmmwh/react-refresh-webpack-plugin#230

[volodymyr-kushnir]

@merceyz Plug'n'Play is what I'm after, so node-modules linker is not an option, but adding missing dependencies might be. Followed your advise and did yarn add --exact eslint-config-react-app, but got another error — Resolve error: unable to load resolver "node" import/no-duplicates — so then I've also yarn add --exact eslint-import-resolver-node and it worked 🎉🥳. Thank you very much 👏!

[stale]

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

[JLarky]

bump

…

On Fri, Dec 25, 2020, 10:10 stale[bot] ***@***.***> wrote: This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#9446 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAABW4UWEMJBF6UCXR7SSMDSWTIS7ANCNFSM4PWYHZHQ> .

[arvinsim]

I just setup create-react-app using Yarn 2 today and I got this error. What is the proposed resolution for this?

[merceyz]

Add it as a dependency until #9872 is merged