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

[New] Add isDirectory; use to speed up node_modules lookups #192

Conversation

keithamus
Copy link
Contributor

@keithamus keithamus commented May 14, 2019

This is a backport of 4cf8928 and fa11d48 (#190 and #191) to the 1.x branch.

This adds the isDirectory option which is needed to drive the directory lookups.

This offers a small but useful performance improvement by avoiding unnecessary stat calls.

Because of the added option this is a MINOR change.

Refs #116

ljharb and others added 2 commits June 16, 2018 15:08
Backport of 698a3e1 to 1.x without the breaking change.

See browserify#154.
This is a backport of 4cf8928 and
fa11d48 (browserify#190 and browserify#191) to the 1.x branch.

This offers a small but useful performance improvement by avoiding unnecessary stat calls.
@ljharb
Copy link
Member

ljharb commented May 14, 2019

698a3e1 was a breaking change, which is why it wasn't backported; it seems strange to have an isDirectory option but not check the basedir opt with it.

@keithamus
Copy link
Contributor Author

Yes, I had realised that the checking of basedir was a breaking change.

We can remove the isDirectory option if you'd prefer, but I wanted to add it because it can be useful to override it with a memoized version for performance reasons (which is exactly what rollup-plugin-node-resolve does).

The readme does not allude to why this module needs these options, it just specifies what they do; therefore it seems to me they are a power-user feature and so users would need to read the code to see how they're used anyway. It would perhaps violate the principle of least surprise to see it used in more places from the jump from 1.x to 2.x but... semver major, caveat emptor.

@ljharb ljharb force-pushed the new-add-isdirectory-use-to-speed-up-node-modules-lookups branch from 7b166f2 to d2816d8 Compare May 15, 2019 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants