-
-
Notifications
You must be signed in to change notification settings - Fork 15
Replace various packages with faster, smaller alternatives #171
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
Comments
I’ll re-review them and see if I want to switch over if it makes sense. |
Colors are built into node.js core since v20.12.0: https://nodejs.org/api/util.html#utilstyletextformat-text-options |
Regarding picomatch I agree, and when we bump lowest supported version of Node.js to 22, then we can use the built in globbing hopefully: #146 (comment) |
Yes but sadly the API design is quite a bit worse than the current color packages, while also being slower and less tested by the community. 🙁 ansis has a great readme comparing all the different alternatives and showing pros and cons: |
@beeequeue Looks like the picomatch swap is causing issues:
Any suggestions on how to fix those? If not, we'll probably have to roll that specific change back until we do. |
Seems related to the use of |
I requested those that ran into issues to re-test on 8.0.4-beta.0. |
Curious Q here. Why does file size matter for this non-client side package? This PR saves a total of say 386kb raw, so like 0.15s on 4G LTE. However, packages are downloaded as tarballs so compressed (.tgz) more and then cached locally through pnpm/npm/github-actions etc. Here the cost, compatibility issues and inconsistencies, doesn't seem to be worth the reward on a dev local kind of thing. |
Got confirmation 8.0.4-beta.0 is working for downstream users who had issues. Since we already paid the cost on this one mostly, I'll give it one more go but if we run into more issues with it, I think we're rapidly entering a cost/value tradeoff that isn't worth it. I would very much rather spend maintenance time on this project on higher value issues like getting type checking set up, modernizing the codebase or speeding up the test suite vs dependency swaps of increasingly little value over the ones we've already landed. |
Not totally unrelated: #181 |
Uh oh!
There was an error while loading. Please reload this page.
Hi, would you be open to a PR that replaces some of the libraries used in this package with faster, smaller alternatives?
The ones I have my eyes on are:
ansi-styles
->ansis
read-package-json-fast
->empathic
+JSON.parse(readFileSync)
read-package-json-fast
except opening+parsing the fileminimatch
->picomatch
cross-spawn
->tinyexec
/nano-spawn
>=20.19
The text was updated successfully, but these errors were encountered: