pulsar-edit
ppm: Install with the `yarn install --production` flag
Saves ~5-7MB disk space, depending on how you count it. Should make the packaged Pulsar binaries slightly smaller as well. Reduces file count, may marginally improve install times of Pulsar due to reduced disk I/O.
Description of the Change
Install the ppm dir with yarn install --production (omits devDependencies), rather than basic yarn install (which installs both dependencies AND devDependencies).
Quantitative Performance Benefits
Measuring by disk usage of the ppm dir, using ncdu.
- Before:
Total disk usage: 213.6 MiB Apparent size: 168.0 MiB Items: 20613 - After:
Total disk usage: 206.4 MiB Apparent size: 163.2 MiB Items: 19441
About 5-7 MB of disk savings (depending on how you measure it), 1172 fewer files/directories.
Possible Drawbacks
This omits the ~~coffeescript and~~ coffee-lint dependencies, among others. I don't think those (or anything in devDependencies are actually used in production. [ EDIT: actually, coffee-script is still there, since ppm's dependency season depends on it. ]
Risk seems minimal, but if we missed where these things are needed, it would cause bugs.
Note: apm and the Atom-community fork used to publish and install apm as a package from the npm package registry. This implicitly did not include any of the devDependencies as well. So I feel that proves out that the devdependencies are NOT needed in production, or at least they weren't needed as of then. I don't think we've mistakenly relied on a devDependency of ppm in production since those times.
Verification Process
Admittedly I haven't really tested this, but I can take some time to do so at some point.
Applicable Issues
None
Release Notes
Install ppm with the yarn install --production flag (save some disk space by excluding devDependencies)
Note that we could further save space and file count by omitting the spec/ dir (2.7 MB with 256 files/subdirs), script/ dir (8 files), native-module dir (5 files and a directory, this should probably be in spec/fixtures anyway)...
The above is roughly how much stuff got pruned out for upstream official Atom or for the atom-community fork.
Removing the coffeescript dependency seems risky to me. There are old packages that are written in CoffeeScript and we should make 100% sure that those will still work. I imagine some users also still have init.coffee files rather than init.js files.
(That said: if we turn out to need coffeescript for those reasons, then it should just be moved to dependencies. This PR is a good idea regardless.)
tl;dr: coffee-script isn't actually removed (whoops, I misspoke), and also this is just ppm's dependencies, not removing anything from under the main core repo's primary node_modules dir or anything like that.
@savetheclocktower thanks for raising those concerns.
I was a bit mistaken in my initial communication, besides probably being unclear what this PR was for.
First, for context, this is specifically optimizing how we install the core repo's copy of ppm. This PR doesn't touch the main dependencies of core repo, just the expected packages in ppm/node_modules when installing core's copy of ppm.
Also: it turns out I was mistaken about coffeescript actually being removed.
The diff from the first page of ncdu output looking at the top level of ppm's node_modules dir looks like this:
@@ -6,14 +6,12 @@
2.0 MiB [ ] /es5-ext
1.8 MiB [ ] /second-mate
1.3 MiB [ ] /path-scurry
- 1.2 MiB [ ] /requirejs
1.2 MiB [ ] /object.assign
1.1 MiB [ ] /superagent
1.1 MiB [ ] /ajv
1.1 MiB [ ] /async
940.0 KiB [ ] /escodegen
824.0 KiB [ ] /make-fetch-happen
- 800.0 KiB [ ] /coffeelint
732.0 KiB [ ] /ext
728.0 KiB [ ] /libnpx
708.0 KiB [ ] /bluebird
@@ -22,10 +20,8 @@
584.0 KiB [ ] /uri-js
556.0 KiB [ ] /request
528.0 KiB [ ] /libcipm
- 524.0 KiB [ ] /coffeestack
516.0 KiB [ ] /nan
516.0 KiB [ ] /JSONStream
- 512.0 KiB [ ] /jasmine-node
504.0 KiB [ ] /acorn
500.0 KiB [ ] /get-uri
492.0 KiB [ ] /vscode-oniguruma
@@ -35,14 +31,18 @@
440.0 KiB [ ] /coffee-script
432.0 KiB [ ] /foreground-child
428.0 KiB [ ] /formidable
- 428.0 KiB [ ] /fileset
424.0 KiB [ ] /resolve
424.0 KiB [ ] /type
416.0 KiB [ ] /node-addon-api
396.0 KiB [ ] /iconv-lite
396.0 KiB [ ] /@iarna
380.0 KiB [ ] /es6-promise
- 364.0 KiB [ ] /express
340.0 KiB [ ] /npm-lifecycle
324.0 KiB [ ] /esprima
- Total disk usage: 132.9 MiB Apparent size: 88.6 MiB Items: 20201
+ 308.0 KiB [ ] /tar
+ 300.0 KiB [ ] /@isaacs
+ 296.0 KiB [ ] /bl
+ 292.0 KiB [ ] /sshpk
+ 280.0 KiB [ ] /vm2
+ 280.0 KiB [ ] /yargs
+ Total disk usage: 125.7 MiB Apparent size: 83.9 MiB Items: 19029
There are more packages than this, but this is the view I was looking at. (First page of a console-based disk usage analyzer's output.) I believe I mis-skimmed/misinterpreted the removal of coffeelint and coffeestack as being a removal of coffeescript itself.
You can see coffee-script (old package name with the hyphen) is still present/unchanged in the above diff. yarn why coffee-script shows the following...
% yarn why coffee-script
yarn why v1.22.19
[1/4] 🤔 Why do we have the module "coffee-script"...?
[2/4] 🚚 Initialising dependency graph...
[3/4] 🔍 Finding dependency...
[4/4] 🚡 Calculating file sizes...
=> Found "[email protected]"
info Has been hoisted to "coffee-script"
info Reasons this module exists
- Specified in "devDependencies"
- Hoisted from "jasmine-focused#jasmine-node#coffee-script"
- Hoisted from "season#cson-parser#coffee-script"
info Disk size without dependencies: "452KB"
info Disk size with unique dependencies: "452KB"
info Disk size with transitive dependencies: "452KB"
info Number of shared dependencies: 0
=> Found "coffeelint#[email protected]"
info This module exists because "coffeelint" depends on it.
=> Found "coffeestack#[email protected]"
info This module exists because "jasmine-focused#jasmine-node#coffeestack" depends on it.
✨ Done in 1.07s.
Still there in --production mode, since season (the CSON parser library, I believe?) depends on it.
[ That said, I'm not clear if ppm needs coffee-script in production other than to read CSON files? Since it doesn't need to actually parse any CoffeeScript during run-time, as far as I know? It might copy some CoffeeScript files from the package templates, but without needing to understand/parse the files it's copying? But this bit is effectively mildly off-topic, since I don't think this question affects core repo, per se. ]
I was a bit mistaken in my initial communication, besides probably being unclear what this PR was for.
Nah, you indicated it was PPM in the title. I just didn't read closely enough.
I'd really like us to see why one of our CI runs is failing here.
Same, that's kind of making me scratch my head. A little nervous to merge this with CI failing for no clear reason as of yet.
I am also leaning toward holding off merging for now, honestly. Wanna sort that out.
The really strange thing is, this PR running yarn run build:apm, effectively cd ppm && yarn install --production...
And it's doing yarn install --production in the main dir instead of the ppm dir, thus breaking the build, because all the devDependencies of the main app are in fact missing.
So much for "not removing anything from under the main core repo's primary node_modules dir or anything like that."
This works for me on my local machine, not sure what about putting it in the CI workflow definition and doing it on a CI machine makes it not run in the correct directory.
Note that we could further save space and file count by omitting the
spec/dir (2.7 MB with 256 files/subdirs),script/dir (8 files),native-moduledir (5 files and a directory, this should probably be inspec/fixturesanyway)...The above is roughly how much stuff got pruned out for upstream official Atom or for the atom-community fork.
@DeeDeeG Just taking a look around, and studied up on this one.
Seems yarn may have a built in flag that can help us here.
yarn --cwd ./ppm install --production
Just tested this one locally, and it does in fact seem to work as expected!
CI failed againwith the same issue, yarn is installing the main dir's dependencies in --production mode, so we miss needed devDependencies in main dir and I'm unclear of ppm ever gets installed.
Perhaps there is some weird env var set in CI that is interfering with how Yarn works, or some other odd aspect of the CI environment vs our development environments.