Skip to content
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

Idea: reducing size of node_modules with postinstall script #1021

Closed
wants to merge 1 commit into from
Closed

Idea: reducing size of node_modules with postinstall script #1021

wants to merge 1 commit into from

Conversation

vanodevium
Copy link

@vanodevium vanodevium commented Oct 1, 2021

Looks like we can reduce the size of node_modules by ~40 percent

All we need is to delete necessary folders:

  • node_modules/rxjs/src
  • node_modules/rxjs/bundles
  • node_modules/rxjs/_esm5
  • node_modules/rxjs/_esm2015

Without postinstall script we have

❯ npm install --only=prod --quite

added 105 packages, and audited 106 packages in 2s

17 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

❯ du -sh node_modules
23M     node_modules

Pay attention to 23M. This is size of node_modules.

With simple postinstall command, we have

❯ npm install --only=prod --quite

> [email protected] postinstall
> rimraf node_modules/rxjs/src node_modules/rxjs/bundles node_modules/rxjs/_esm5 node_modules/rxjs/_esm2015


added 105 packages, and audited 106 packages in 2s

17 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

❯ du -sh node_modules
14M     node_modules

Pay attention to 14M. This is size of node_modules.

cc @ai

@ai
Copy link
Contributor

ai commented Oct 1, 2021

It can be dangerous to change other package. It can be re-used by somebody else.

@vanodevium
Copy link
Author

Yeap, but these folders are really useless for node environment.
Also, local tests are passed with no errors.

In my opinion, there are no other ways to reduce node_modules of lint-staged

@okonet
Copy link
Collaborator

okonet commented Oct 1, 2021

May I ask first why is this such a big issue? 23 MB vs 14 MB doesn't look like a big deal to me in the first place. What problem are you trying to solve here?

@vanodevium
Copy link
Author

I only try to reduce size of node_modules with small steps. Step by step.
According to the described situation

@okonet
Copy link
Collaborator

okonet commented Oct 1, 2021

Maybe @ai can explain what’s the goal here and why this is important?

@ai
Copy link
Contributor

ai commented Oct 1, 2021

The problem is that lint-staged has big installation size

https://packagephobia.com/result?p=lint-staged

But applying patch to subdependencies looks like too dangerous way. It is better to replace them to smallest analogs.

@vanodevium
Copy link
Author

@ai
As I understand it, listr2 is one of the most heavy requirement because it has a very heavy rxjs.
Does it make sense to try to fix this dependency in an honest way (without postinstall patching)?

@ai
Copy link
Contributor

ai commented Oct 1, 2021

Yeap. listr subdependencies. The author is very open.

@iiroj
Copy link
Member

iiroj commented Oct 2, 2021

This PR #1003 has some improvements regarding this, since Listr2 is updated to latest version, the direct depency to chalk is replaced with colorette and supports-color, and finally log-symbols is removed because Listr2 now exports its own figures.

@iiroj
Copy link
Member

iiroj commented Oct 2, 2021

I think this PR shouldn't be merged because it seems very dangerous to remove local node_modules. Let's try to cut down on dependency size by actually updating/replacing them rather than things like this!

Closing the PR.

@iiroj iiroj closed this Oct 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants