Contributing: Decrease the size of the node_modules needed to be downloaded

[kevin940726]

Similar to #29008.

Is your feature request related to a problem? Please describe.

As pointed out by @tellthemachines initially, the size of the node_modules needed to be downloaded is really huge for regular contributors. It might be an issue for contributors with really high-end devices, but it certainly could be a problem for contributors with lower-end devices.

Running npm install on master produces the following result:

$ npm install
$ du -hs node_modules
1.6G    node_modules

1.6G of node_modules!

Let's further investigate the contents of the gigantic node_modules folder.

$ du -hs node_modules/* | sort -hr | head -n 20
214M    node_modules/appium
151M    node_modules/@storybook
139M    node_modules/hermes-engine
 72M    node_modules/nodegit
 67M    node_modules/react-native
 57M    node_modules/typescript
 46M    node_modules/@babel
 39M    node_modules/jsc-android
 32M    node_modules/@jest
 23M    node_modules/react-dev-utils
 21M    node_modules/enzyme
 20M    node_modules/metro-inspector-proxy
 18M    node_modules/@icons
 17M    node_modules/rxjs
 17M    node_modules/inquirer
 16M    node_modules/@types
 15M    node_modules/wait-on
 13M    node_modules/prettier
 12M    node_modules/metro
 10M    node_modules/stylelint

We can see that most of the hugest packages are only for testing or native bindings (appium, hermes-engine, react-native, jsc-android, metro...). We don't need them for regular development. In fact, we tend to forward them to CI when opening PRs anyways.

Describe the solution you'd like

Move those packages which don't need to be downloaded for regular development to optionalDependencies. Then defaults to install packages with optional=false.

This way, regular installs (npm install) won't download them, resulting in smaller size of node_modules needed. We can still install them via npm install --optional when necessary or in CI environment.

Caveats

• optionalDependencies isn't really optionalDevDependencies though, but it's the best we can get.
• Not decreasing the size enough though, but still are improvements.
• What does it mean to the native developers? Are we sacrificing their DX for ours?

Describe alternatives you've considered

None that I can think of.

[gziolo]

@hypest and @Tug, would you mind sharing your thoughts from the perspective of the WordPress Mobile team?

[Tug]

Are we talking about having optional=false in the .npmrc file or just letting the user choose to install only the required dependencies (npm install --no-optional)?

The latter would be fine I guess. However the former case would force native developers to change their flow (as well as adapt the CI jobs). But worse, it would not help our cause which is to make it easier for everyone to contribute to native development.

[hypest]

Yikes, the Appium dependency does have a big footprint! Not sure if there's some optimization or refactor we could do to reduce it without removing it or making its integration hard to work with.

On the other hand, the Hermes dependency is also big but that might have an easy "fix" by updating to newer versions (this commit reduces the footprint to a couple dozen of MBs apparently).

In general, although it's a fair goal to look for opportunities to reduce node_module's (and download size) footprint, the target also needs to be to help everyone test their changes on all platforms, including native mobile. In this case, Appium is the tool to run the native mobile end-to-end tests on devices or emulators so, it's an important dependency.

Playing devil's advocate for a bit, we could also argue that the big Storybook dependency is not needed for native mobile devs so we should try to reduce or defer its installation. I don't think that's a good argument.

What I'm trying to say: let's keep shooting for tighter integration and better tooling for developing and testing across platforms, maintaining or improving the DX for all devs.

[azaozz]

Corresponding ticket for core trunk: https://core.trac.wordpress.org/ticket/51784.

Are we talking about having optional=false in the .npmrc

Yeah, was testing with adding it there in core trunk. Would that also be bad/unacceptable?

[kevin940726]

let's keep shooting for tighter integration and better tooling for developing and testing across platforms, maintaining or improving the DX for all devs.

Yeah, it makes perfect sense! That's what I listed in the #caveats part too.

Sadly, npm doesn't support a mode or some feature like that to only install certain subsets of the dependencies for different devs 🤔 .

Is listing some dependencies in optionalDependencies still useful in this case? Or should we just close this?

[hypest]

Thanks for the understanding @kevin940726!

Is listing some dependencies in optionalDependencies still useful in this case? Or should we just close this?

"Optional dependency" gives the wrong signal/message, IMHO. Those dependencies are not really optional.

That said, if we don't have another way and the issue is blocking some contributors, then I'd say it should be OK if someone implements this and document it for folks. Important: we should not default to not installing the optionals though. Contributors using that flow will need to run npm install --no-optional themselves (just like @Tug proposed earlier), and since that can cause build errors, either someone needs to adapt and maintain the code to cater for optionals, or contributors might reach deadends with build errors or failing tests jobs.

[azaozz]

...since that can cause build errors

How about there is a brief "help" outputted in the console? Was looking at that for core trunk too. Can even be a prompt (Y/N), etc. and of course there should be a way to bypass it with a switch?

To me it seems npm still needs to "grow up" a bit and handle all these cases better. It is, after all, a package manager, and there are some fine examples of what package managers can do :)

[hypest]

How about there is a brief "help" outputted in the console? Was looking at that for core trunk too. Can even be a prompt (Y/N), etc. and of course there should be a way to bypass it with a switch?

Anything that could ease the use of that alternative workflow (avoid optionals) and make it robust would be helpful, I guess.

This could be a reminder/note for the folks that will pick this up.

[gziolo]

$ npm --version 7.5.2 $ npm i npm ERR! code ENOENT npm ERR! syscall chmod npm ERR! path /Users/gziolo/Projects/gutenberg/node_modules/appium/node_modules/.bin/authorize-ios npm ERR! errno -2 npm ERR! enoent ENOENT: no such file or directory, chmod '/Users/gziolo/Projects/gutenberg/node_modules/appium/node_modules/.bin/authorize-ios' npm ERR! enoent This is related to npm not being able to find a file.

It would be great to update appium to the latest version and make it optional – it would install when executed for the first time. I can't finish npm i with the latest version of npm.

[hypest]

It would be great to update appium to the latest version

Just sharing here that @fluiddot is on it as we speak.

[gziolo]

This is how the Chrome binary is being installed when running e2e tests for the first time:

https://github.com/WordPress/gutenberg/blob/a1d586017453cbff6037fd573950e8f491f35a09/packages/scripts/scripts/test-e2e.js#L31-L33

I have no idea how appium is architected, but you could try using execa to install it without modifying the package.json file when the tests are fired, for example:

https://github.com/WordPress/gutenberg/blob/a1d586017453cbff6037fd573950e8f491f35a09/packages/create-block/lib/templates.js#L148-L155

(it doesn't have to be the temp folder, it's necessary there for different reasons)

[fluiddot]

It would be great to update appium to the latest version

Just sharing here that @fluiddot is on it as we speak.

Thanks for pointing me this out, I have a WIP PR for updating Appium, so I could also include making it optional 👍 .

[gziolo]

I raised a similar issue on WordPress Slack last Friday (link requires registration at https://make.wordpress.org/chat/): https://wordpress.slack.com/archives/C02QB2JS7/p1613054540483100

I did the screenshot just after the fresh install.

[gziolo]

I wanted share an excellent article on the topic from @cpojer:

https://cpojer.net/posts/dependency-managers-dont-manage-your-dependencies

One of many things we should consider from the article is setting up CI job to prevent node_modules growing further (cc @ockham):

It’s great to reduce the number and size of dependencies once, but it’s best to sustain the wins long-term. I recommend setting up a CI step that analyzes the size of node_modules using something like du -sh node_modules within a project whenever yarn.lock or package.json files are changed. Make the CI step run on every Pull Request, and if the size increases compared to master, alert somebody to take a look.

Christoph also commented on my tweet with an interesting solution to consider:

For RN, both JSC and Hermes are big packages. If you are only using one, you can „blackhole“ the other one by using resolutions to an empty package. Saves you like 80 MiB!

[hypest]

Christoph also commented on my tweet with an interesting solution to consider:

For RN, both JSC and Hermes are big packages. If you are only using one, you can „blackhole“ the other one by using resolutions to an empty package. Saves you like 80 MiB!

That's a cool idea! Probably not applicable yet though as we're using Hermes on Android only. Down the road, I expect to use Hermes on iOS too at which point I assume it will be doable to keep installing the Hermes package only.