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

perf(npm): use direct binary for startup boost #839

Closed

Conversation

dalisoft
Copy link

@dalisoft dalisoft commented Apr 7, 2024

This changes/PR should improve startup performance of CLI by up to 5 times and very huge impact for lint-staged, pre-commit and pre-push commits


This PR conflicts with #840 as this PR uses different approach to use binary

Use this or #840
Approve and merge whichever ways you want/like

Added support for arm64 on npm build.ts

@CLAassistant
Copy link

CLAassistant commented Apr 7, 2024

CLA assistant check
All committers have signed the CLA.

@dalisoft

This comment was marked as outdated.

@dalisoft dalisoft changed the title perf(npm): use direct binary for huge startup boost perf(npm): use direct binary for startup boost Apr 7, 2024
"scripts": {
"postinstall": "node ./install.js",
"postinstall": "node ./postinstall.js",
Copy link
Member

Choose a reason for hiding this comment

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

I think this now requires the post install step? Previously dprint would work even when someone did npm install --ignore-scripts.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it is true. This is cost for performance improvement

Copy link
Author

@dalisoft dalisoft Apr 15, 2024

Choose a reason for hiding this comment

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

@dsherret Or try #840. #840 should work with ignoring scripts but not sure if npx/bunx work

@dalisoft dalisoft closed this Apr 24, 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.

None yet

3 participants