typescript-eslint
Repo: Use node native methods instead of extra libraries when possible
Suggestion
I noticed that there is update for make-dir, which lead me to check where it's needed. Only place where it's used is generate-website-dts.mts (https://github.com/typescript-eslint/typescript-eslint/blob/2f0c6ac62b797e2e57393962ae315078c6811a03/packages/website/tools/generate-website-dts.mts#L4) . For me it looks like it could be replaced with node's own mkdir.
Same script uses rimraf, and probably could use node's own methods. rimraf is used elsewhere too, so removing it's usage from that script file won't remove the need for that dependency.
Also, for me, it looks like cross-fetch is used in places, where node's fetch could be used too.
--
I truly understand, if you feel that the benefit for this change would not be greater than the work needed for code review and hassle. The biggest benefit for maintainers would probably be just having less renovate PRs.
It's possible that I also overlooked some requirements, why these external libraries are needed.
Additional Info
No response
If rimraf is only used for scripts after this, maybe it's worth swapping to premove too as it has less dependencies. (Not that it matters much in practice as it's local dev only, but make things cleaner and reduce potential attack surface)
We're pretty burned out on e18e-style "switch to a smaller equivalent" initiatives right now: see https://github.com/typescript-eslint/typescript-eslint/issues/11212#issuecomment-3010281265 -> after the ---. But removing a dependency in favor of Node.js-native methods is always nice. Let's wait until November 1 per the comment and see how things are then.
That’s totally understandable!
When triage is open, my summarized proposal is: remove usage of make-dir, cross-fetch and rimraf from repo script files. In this time, do not touch usage in package.json files.
(I don’t see much benefit of changing rimraf to premove right now.)
OK! Coming back to this: using Node.js-native methods generally sounds great. Direct win. As long as CI passes and the code isn't made more complex (i.e. we don't end up just re-implementing userland wrappers around Node.js primitives) I'm in favor.
The only tricky point to note is that we're still running on Node.js 18 and 20 in CI:
https://github.com/typescript-eslint/typescript-eslint/blob/740a63fd3e0fe2d9c30ae5eff6a41b0941a3129b/.github/workflows/ci.yml#L19
https://github.com/typescript-eslint/typescript-eslint/blob/740a63fd3e0fe2d9c30ae5eff6a41b0941a3129b/.github/workflows/ci.yml#L190
From https://typescript-eslint.io/users/dependency-versions/#node:
The version range of NodeJS currently supported is
^18.18.0 || ^20.9.0 || >=21.1.0.
It'd be breaking change to drop support for any of those. Our next major version isn't coming anytime soon. So any Node.js-native methods would need to work in all CI tasks that run them. Meaning:
- User-facing code needs to work for Node.js versions in that range for now
- Internal-only code needs to work for Node.js versions that run that code in CI (which is 18 for some, or only newer versions for others)
Accepting PRs for changes that meet those criteria. If there's any Node.js-native method that's, say, blocked on dropping 18.x, that can be a followup issue.