Skip to content
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

Improvement idea - globally configurable helper functions #280

Open
naugtur opened this issue Apr 22, 2022 · 3 comments
Open

Improvement idea - globally configurable helper functions #280

naugtur opened this issue Apr 22, 2022 · 3 comments

Comments

@naugtur
Copy link

naugtur commented Apr 22, 2022

I was diagnosing performance of an app scanning node_modules. It turned out half of the time is spent in isDirectory on the few cases where it was looking for a missing folder.

What helped was passing in an isDirectory implementation that bails out if the path has gone out of the current folder.

For cases of using resolve hundreds of thousands of times in a build process etc. doing a full tree folder listing and caching it would provide a great deal of performance improvement.

For that purpose, setting isDirectory and other helper functions on the resolve module singleton instead of drilling them down to where it's called would be useful.

Is that an interesting/welcome contribution?

@ljharb
Copy link
Member

ljharb commented Apr 22, 2022

Statefulness wouldn’t be an improvement; but I’m pretty unclear on why they’d need to be stored on the function.

You could certainly make a wrapper module that has a stateful isDirectory as well.

Performance improvements that don’t involve state are most welcome.

@naugtur
Copy link
Author

naugtur commented Apr 23, 2022

I don't mean adding state but exposing the default implementations of helper functions so they can be overwritten.
My previous description was... written in a rush and a bit messy.

If I could do resolve.defaults.isDirectory = myfunc that'd be it

If that's also considered unwanted state, I'm convinced I need to work on a different angle for this. Maybe I'll monkey patch fs 😉

@ljharb
Copy link
Member

ljharb commented Apr 23, 2022

That would still be state, and it would break other consumers just as if you modified Array.prototype.

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

No branches or pull requests

2 participants