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

feat(puppeteer): Cleanup node_modules #1497

Merged
merged 3 commits into from
Jul 24, 2024
Merged

Conversation

lslezak
Copy link
Contributor

@lslezak lslezak commented Jul 24, 2024

Problem

  • The Puppeteer node_modules directory is huge

Solution

  • Delete the not needed content from node_modules
  • Inspired by node-prune tool (written in Go, not much suitable to run during RPM build, I rewrote that to shell)

Testing

  • Tested manually

Numbers

  • The node_modules directory size went down from 39MB to 15MB (-62%)
  • The built RPM size went down from 3.5MB to 1.9MB (-46%)
  • I expect the Live ISO size reduction similar to the RPM reduction

LICENCE
LICENCE-MIT
LICENCE.BSD
licence
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that list is just copied, but can we legally delete such license files? maybe if there are identical, we can just run deduplication in rpm build?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, good point. As we already run the deduplication in RPM we could keep them. I'll check how much the size changes...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With included license files the RPM size increased by about 24kB, so I'll rather keep them.

MODULES_PATH="${1:-./node_modules}"

# delete Puppeteer CommonJS modules, we use the ES modules (in lib/esm)
rm -rf "$MODULES_PATH/puppeteer-core/lib/cjs"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of expect that it will run internally that node-prune.sh..so just one command will be enough to cleanup pupeteer modules.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both commands are started in the agama-integration-test scripts which installs the NPM packages and runs the tests. Normally you should not need to run them separately.

@@ -47,6 +47,12 @@ outside.
rm -f package-lock.json
local-npm-registry %{_sourcedir} install --omit=optional --with=dev --legacy-peer-deps || ( find ~/.npm/_logs -name '*-debug.log' -print0 | xargs -0 cat; false)

# node_modules cleanup
%{_builddir}/agama/node-prune.sh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we run it also for agama web directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agama-web uses webpack which already bundles only the needed files into the target JS file. It would not help there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And we do obfuscation and minification as well...

@lslezak lslezak merged commit 10d6157 into master Jul 24, 2024
@lslezak lslezak deleted the puppeteer_node_modules_cleanup branch July 24, 2024 15:38
@imobachgs imobachgs mentioned this pull request Sep 20, 2024
imobachgs added a commit that referenced this pull request Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants