-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enforce hoisting of some packages to fix giant disk size with Yarn #1397
Comments
I've been looking to get more involved in open source work. I'd be glad to look into this and see if I can fix it. |
Yeah me too. I have created a list of duplicate modules installed by create-react-app, but am not sure what to do next. |
Yes, see instructions here.
Should be. |
Great I'll give it a crack thanks |
According to pkgcount --du --sort size, after a fresh
Other duplicates are far less significant, so deduping |
FYI this is the same issue as npm 2 (why we didn't suggest using npm 2 for Babel 6). Luckily after dropping Node < 4 we should be able to remove babel-runtime (core-js/regenerator) from babel itself after some work in babel/babel#5118. Otherwise workaround is to make babel-runtime an explicit dependency for now. (This is because almost all the babel plugin packages have babel-runtime (since we use https://babeljs.io/docs/plugins/transform-runtime/) to support Symbol, Promise, Map/Set, etc which are in Node 4.) And as for a timeline we are just going to do another 6.x + fixes and 7.x work is already starting https://github.com/babel/babel/wiki/Babel-7 thanks to contributors! The scope of the changes isn't huge so it should be an easy upgrade (at least I intend it to be) |
I don't mean to imply that I'm somehow more qualified to prioritize tasks than anyone else, especially @gaearon, but I fail to see why this issue is a big deal. At the end of the day, disk space is cheap, this has no impact on build size or, if I understand correctly, build time. Its just feels to me like complaints about the size of node_modules are almost like saying "back in my day we used to walk uphill both ways to hand pick minified jQuery plugins and lovingly lay them down in the Certainly this duplication bug is a problem that yarn should strive to fix, but I don't really see how this matters enough to warrant temporary workarounds in CRA. |
Disk space may be cheaper but it's not universal truth especially for older machines running out of space. If you work on many projects it's annoyingly each each of them takes a whole gig. It also impacts install times negatively because the linking step takes a long time. |
I was just tracking down where the I have no idea how feasible it would be to update |
I can't categorically disagree with anything you've said, but I just don't think that annoyance is enough of a reason to "dirty" react-scripts' package.json with incorrect dependencies, let alone disable yarn integration entirely. |
@mmgm The link time alone is a bit of a killer for me, taking up to 2 minutes for an app that really isn't so big. I'm on a not so powerful laptop but it's still a little crazy. |
@FWeinb oh interesting. Sounds like a similar issue with when Babel 6 was still being compiled with Babel 5 causing core-js@1 and core-js@2 deps and fixed with babel/babel#3438 (to have the same core-js/babel-runtime version) Also FYI: babel/babel#3340 for core-js@1 to core-js@2 update in Babel a while ago. |
@hzoo |
So I've done some investigation into deduping some of the dependencies: Baseline:
Adding --- a/packages/react-scripts/package.json
+++ b/packages/react-scripts/package.json
@@ -28,6 +28,7 @@
"babel-eslint": "7.1.1",
"babel-jest": "18.0.0",
"babel-loader": "6.2.10",
+ "core-js": "2.4.1",
"babel-preset-react-app": "^2.0.1",
"case-sensitive-paths-webpack-plugin": "1.1.4",
"chalk": "1.1.3",
Also adding --- a/packages/react-scripts/package.json
+++ b/packages/react-scripts/package.json
@@ -28,6 +28,8 @@
"babel-eslint": "7.1.1",
"babel-jest": "18.0.0",
"babel-loader": "6.2.10",
+ "babel-runtime": "6.20.0",
+ "core-js": "2.4.1",
"babel-preset-react-app": "^2.0.1",
"case-sensitive-paths-webpack-plugin": "1.1.4",
"chalk": "1.1.3",
Lastly if just --- a/packages/react-scripts/package.json
+++ b/packages/react-scripts/package.json
@@ -28,6 +28,7 @@
"babel-eslint": "7.1.1",
"babel-jest": "18.0.0",
"babel-loader": "6.2.10",
+ "babel-runtime": "^6.20.0",
"babel-preset-react-app": "^2.0.1",
"case-sensitive-paths-webpack-plugin": "1.1.4",
"chalk": "1.1.3",
Node version: v6.9.0 |
babel-runtime@6.20.0 contains core-js@^2.4.0 so dono if there are other babel-runtime versions as well? regenerator-runtime may help as well but probably not as much (also the other dep in babel-runtime). Either way we will try to fix this soon for Babel 7. |
Sounds awesome, send a PR please. |
Fixed by #1441, will be out in 0.9.0. |
Yarn has known issues deduplicating dependencies, sometimes failing to hoist dependencies shared between a ton of packages (such as
babel-runtime
) and producing installs 5 times bigger than npm: yarnpkg/yarn#2306.While we could disable Yarn integration (#1390), I'd like to explore an alternative approach. We should be able to determine which package causes most duplication, and then hoist it manually by making it an explicit dependency of
react-scripts
. This would ensure Yarn deduplicates it. This is a temporary workaround until Yarn actually fixes the algorithm (tracked in yarnpkg/yarn#2306).Please don't hesitate to help with this. You would need to:
node_modules
of each of them.)react-scripts
itself to hoist them.The text was updated successfully, but these errors were encountered: