browserify / resolve

Implements the node.js require.resolve() algorithm
MIT License
775 stars 183 forks source link

Improvement idea - globally configurable helper functions #280

Open naugtur opened 2 years ago

naugtur commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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