refactor(deps): replace find-up with empathic #5365
Conversation
| @@ -92,7 +92,7 @@ exports.loadConfig = filepath => { | |||
| * @returns {string|null} Filepath to config, if found | |||
| */ | |||
| exports.findConfig = (cwd = utils.cwd()) => { | |||
| const filepath = findUp.sync(exports.CONFIG_FILES, {cwd}); | |||
| const filepath = find.any(exports.CONFIG_FILES, {cwd}); | |||
There was a problem hiding this comment.
In one call, findUp.sync is replaced with find.any, in another, it's replaced with find.up. Is that intentional?
There was a problem hiding this comment.
Ref https://github.com/lukeed/empathic/blob/main/src/find.ts, they look different, can we stick with find.up for now?
There was a problem hiding this comment.
It is intentional, up searches for a single path while any searches for multiple:
find.up(path: string) vs. find.any(paths: string[])
|
Posting here as well as apparently we have two issues and one PR regarding this now, so not sure where to keep the discussion: Swapping a ≈163221k weekly downloads with 5406 dependents for a ≈32k weekly downloads with 10 dependents (and picking up steam very recently) is something that requires quite a bit of consideration and justification. I'm not sure this is the kind of swap that mocha should be spearheading – there are other more cutting edge modules out there that can take the hit first. There's also a lighter alternative of find-up itself that should also be considered: https://www.npmjs.com/package/find-up-simple Remember: The smaller size gained from eg fewer transitive dependencies or a lighter module itself, that's only gained if we are the only package depending on the version of the package that we are swapping from. If it or its transitive dependencies are already installed, then a swap will actually cause a larger install, not a smaller, if the swap is to a library that the typical installation does not already have. So it's not as simple of an equation as simply looking at package size in isolation. |
|
Moving to draft pending discussion in #5346. |
There was a problem hiding this comment.
Unfortunately I have reviewed this at depth and it would cause regressions as well as having dubious performance gains: #5346 (comment)
I see no reason to explore this further. At least not until the upstream package has become a drop-in replacement.
fixes #5346
fixes #5359
PR Checklist
status: accepting prsOverview
This PR swaps
find-upfor the faster and smaller alternativeempathic, see #5346 and #5359 for more details.