es-tooling / ecosystem-cleanup

A place to keep track of ongoing efforts to clean up the JS ecosystem
386 stars 2 forks source link

Migrate `rimraf` to `node:fs` in `firebase-tools` #126

Open echedaleux opened 1 month ago

echedaleux commented 1 month ago

Use built-in Node APIs instead rimraf.

echedaleux commented 1 month ago

PR opened here on the firebase-tools repository : https://github.com/firebase/firebase-tools/pull/7826

Namchee commented 1 month ago

This repository also use the following dependencies:

43081j commented 1 month ago

given they specify:

  "engines": {
    "node": ">=18.0.0 || >=20.0.0"
  },

A few others I noticed:

I suspect if you look at the graph or the pkg-size result, you'll see yaml is pretty chunky too. but I'm not sure if there are alternatives (that may just be how big a yaml parser has to be)

jsaguet commented 1 month ago

PR to remove strip-ansi https://github.com/firebase/firebase-tools/pull/7860

jsaguet commented 2 weeks ago

node-fetch, abort-controller, form-data and proxy-agent dependencies are all linked together.

I've worked on a PR to replace everything by native fetch.

It seems to be working but every test relies on nock which does not support native fetch on the current version (13). Updating everything to undici's MockAgent is a huge work and I'm not sure it is really worth it considering fetch support is planned for the next release (14) of nock.

Are there other examples of migrations from node-fetch to fetch so that I may find another way ?

43081j commented 2 weeks ago

Do you have a link to the branch? And an example test that fails

Maybe we can figure something out. Would be good to see some code though so I can better follow what you mean

jsaguet commented 2 weeks ago

Sure, here is the branch: https://github.com/jsaguet/firebase-tools/tree/feat/remove-node-fetch

My point was that every test in firebase-tools that mocks http requests does it with the nock package. nock works well with node-fetch because both are based on node:http / node:https modules.

However, nock does not work with fetch yet so moving to native fetch breaks all of those tests because network calls are not mocked anymore.

Migrating those tests to another tool would be a huge effort with very low value and the PR might by rejected by the maintainers because they are used to nock.

We could just wait for the next release of nock which will support mocking fetch calls.

I was asking if there was known drop-in replacement to nock that can mock fetch calls because I could easily switch to such tool right now instead of waiting for the next release of nock.

43081j commented 2 weeks ago

That makes sense. Really it would be nice to just stub fetch out like any other function. But it would be a big refactor, you're right

I'm not aware of a nock alternative either unfortunately. Do you know if there's a tracking issue for them supporting fetch? So we can follow

jsaguet commented 2 weeks ago

I found this one https://github.com/nock/nock/issues/2397

43081j commented 2 weeks ago

aha yes it looks like nock@beta might do what you need!

they already shipped experimental support it seems