npm / cacache

npm's content-addressable cache
Other
280 stars 31 forks source link

fix: Remove usage of `p-map` #281

Closed AbhiPrasad closed 5 months ago

AbhiPrasad commented 5 months ago

When investigating reducing the dependency count for the semantic-release library, I discovered that npm has quite the large dependency tree: https://github.com/43081j/ecosystem-cleanup/issues/64

One area of improvement here is to remove the dependency on the p-map library and replace it with a smaller, simpler helper.

Right now cacache relies on p-map@4.0.0, which pulls in aggregate-error@3.1.0. Newer versions of p-map don't pull this in aggregate-error at all, but that is not feasible to use because this cacache is CJS only, and p-map 5.x and above are ESM only.

Given cacache supports "^16.14.0 || >=18.0.0" as per it's package.json engines field, it doesn't make sense for it to pull in a polyfill for aggregate-error (Node.js 15+ adds support for the built-in AggregateError.

To replace pMap which we used from the p-map library, I introduced a mapWithPromise helper. It doesn't have all the functionality of pMap, but it works well for the use cases in lib/verify.js. In addition it should be slightly faster than pMap because of it is simpler. (let me know and I'm happy to bench mark to get exact numbers).

This change should help eliminate 25 KB / 4 deps from the module graph of cacache.

References

Supercedes https://github.com/npm/cacache/pull/266

wraithgar commented 5 months ago

p-map being esm-only does not preclude updating to it here. The blocker is engines. Once we bump that in this repo we can update. Generally we only replace a library and roll our own solution when we absolutely have to. I don't think we will be accepting this PR, sorry.

AbhiPrasad commented 5 months ago

The blocker is engines

Could you clarify why the blocker is engines? Are you waiting for require(esm) support?

wraithgar commented 5 months ago

p-map does not work in node 16, cacache does.

~ $ npm view cacache engines
{ node: '^16.14.0 || >=18.0.0' }
~ $ npm view p-map engines
{ node: '>=18' }