Repository navigation
Don't crash if users mistakingly install babel, webpack, jest, loaders, etc, at the top level #1795
Description
I’ve seen this issue multiple times. For some reason people don’t believe react-scripts is enough and try to install Babel, Webpack, and friends themselves. This can break things.
We need to mitigate this somehow.
Activity
gaearon commentedon Mar 17, 2017
@anyayunli If you’d like control over configs, run
npm run ejectand you have it. This issue is not related to that.langpavel commentedon Mar 17, 2017
This is about inexperienced developers. It can be limited only by more educational examples and strict rules for opening issues.
langpavel commentedon Mar 17, 2017
And more visible dedicated site for users, stack overflow is not enough
Timer commentedon Mar 17, 2017
Is there anything wrong with us showing a nasty-looking warning when people do this?
langpavel commentedon Mar 17, 2017
No, really not. You are experiencing success, you only need way to fastly redirect people from Github to more general discussion about when they should eject!
Timer commentedon Mar 17, 2017
Optimally, we want people not to eject. I feel like there's way too many small things people eject for that's accomplishable without ejecting (e.g. using
sass-loader).langpavel commentedon Mar 17, 2017
So, this can be statistically observed. You can inject some questionary into eject command with helpful links. This is task for @cpojer like person:-)
Timer commentedon Mar 17, 2017
@gaearon what about something like this?
langpavel commentedon Mar 17, 2017
Nice, but not enough. It will works BUT you are not providing hint. When you run eject, it should ask you why you need eject providing most relevant information. No way back after. That's ok, but warn about they are not supported by this awesome toolkit any more..
gaearon commentedon Mar 17, 2017
I’d like us to first look into why this breaks CRA. If there's a version conflict,
react-scriptsshould use its own version ofwebpack. We should fix whatever breaks so that outer version ofwebpackis just ignored. We can optionally warn on top of that but first we should fix the breakage.langpavel commentedon Mar 17, 2017
Different idea. Can you hook yarn with CRA? For example as preinstall, pre-add in package.json. I know, this is something new! But this is in your possibilities!
Or you can create recommended way to install new dependencies with CRA specific cli tool, promote it and drive mainstream. This will give you less observable force than patching yarn. Controversial but, hey, look at real people!
gaearon commentedon Mar 17, 2017
We already use Yarn when it’s installed. It doesn’t solve the problem though.
langpavel commentedon Mar 17, 2017
Yarn hook which will dog watching what you want add
langpavel commentedon Mar 17, 2017
It's entire new space not possible today. Yes you can give recommendations only..:
You just installed package xyz. Did you consider this way? Http....
5 remaining items
gaearon commentedon Jan 8, 2018
No idea, but if you can create one that would be great. My impression is you need to create a project and then install
eslint@4,babel@5,jest@16, etc, and try to trigger it this way.gaearon commentedon Jan 8, 2018
Here is an example, updating
jestto22might reproduce this.jestjs/jest#5119 (comment)
omeid commentedon Jan 8, 2018
Any solution to this would require CRA to work around how the package manager works, which in my opinion is not the right thing. A package should not override the package manager rules.
I am okay with a mechanism that detects wrong versions of the packages and warn, or at worst, just give up.
viankakrisna commentedon Jan 8, 2018
experimented using pkg to package react-scripts as an executable: https://github.com/viankakrisna/react-scripts-packaged/releases/tag/0.0.1
jlongster commentedon Jan 9, 2018
I personally think Jest should be excluded from this list. Using testing through CRA is optional. I'd like to use CRA for all the tooling to setup and run and app, but do testing myself (I like to interact with jest directly).
Maybe the create-react-app script to run tests could do this check, so you would only fail if you locally had jest installed and tried to run tests through CRA. Maybe that's how it's intended to work and I missed that point. :) If so, that sounds great.
gaearon commentedon Jan 9, 2018
I think we keep talking past each other. Sorry if I'm being confusing.
I don't propose to forbid using your own version. (Early wording says that but my more recent comments don't.)
What I'm saying is we should make sure that installing arbitrary Jest/Babel/Whatever version at top level does not break any of built-in
react-scriptsfunctionality. So you should be able to use them side by side if you know what you're doing. In fact that's how I'd expect it to already work—I have no idea why it breaks (according to reports).Does this clarify things? That's the only problem I'm referring to in this issue.
jlongster commentedon Jan 9, 2018
I may have missed a reply, especially since there were two threads that got merged. Sounds great. Sorry if I didn't read the original description well enough.
gaearon commentedon Jan 12, 2018
I did some debugging. The Jest issue is caused by how Jest packages communicate: jestjs/jest#5294.
[-]People try to install babel, webpack, loaders, etc, at top level[/-][+]Don't crash if users mistakingly install babel, webpack, jest, loaders, etc, at the top level[/+]gaearon commentedon Jan 12, 2018
For Webpack, this happens (at least with npm) because:
node_modules/react-scriptsimportsnode_modules/html-webpack-plugin(which npm hoists at the top for some reason)node_modules/html-webpack-pluginimportswebpackand gets the top-level "wrong"node_modules/webpackrather than the intendednode_modules/react-scripts/node_modules/webpackI don't know why
html-webpack-plugingets hoisted but I also don't quite see how to avoid it. For example in monorepos it's expected more things get hoisted.gaearon commentedon Jan 13, 2018
I chatted to npm folks about #1795 (comment) — it is a known npm issue. Yarn doesn't have it. They tried to fix it in 5.x but that created other issues so they're holding off until future majors.
gaearon commentedon Jan 13, 2018
#3771
gaearon commentedon Jan 13, 2018
Fixed by #3771.
There is a way to opt out of the preflight check for those who know what they're doing (it's mentioned at the end of the displayed message).