Skip to content

perf: switch to empathic find package traversal #1053

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

43081j
Copy link

@43081j 43081j commented May 6, 2025

Uses empathic/find for finding the closest package.json, a much faster and smaller library than the various over-specialised alternatives lying around.

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Docs update
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

We traverse the file system for the closest package.json-containing directory.

What is the new behavior?

N/A

Does this PR introduce a breaking change?

  • Yes
  • No

Uses `empathic/find` for finding the closest `package.json`, a much
faster and smaller library than the various over-specialised
alternatives lying around.
@JLHwung
Copy link
Contributor

JLHwung commented May 6, 2025

Thank you. find-up is only executed once when Babel-loader computes the default cache directory for any later cache calls. Since find-up is sufficiently fast for our use case, performance-wise switching to empathic will not have a very big impact.

find-up is also used by other popular tools such as ESLint and c8. So it is very likely that find-up already presents in users' node_modules. We will consider the switch if other tools also switches to empathic.

@43081j
Copy link
Author

43081j commented May 6, 2025

Over time, the de-dupe will switch since we're in the process of moving many other popular tools to do this switch too.

you're right that the performance bump won't be much here since it isn't called often, but it is greater than none. there's currently no reason to not do this switch other than doubt that the de-dupe will be improving, but we have to start somewhere.

what you should be aware of is that empathic replaces many other packages in other repos (there are many different npm packages doing roughly the same thing). eventually, we will be installing this one package instead of many, across a given installation. babel-loader is just one part of it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants