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

Projects
None yet
2 participants
@keithamus
Copy link
Contributor

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 some commits Jun 16, 2018

[New] add `isDirectory`
Backport of 698a3e1 to 1.x without the breaking change.

See #154.
[Performance] use `isDirectory` to speed up `node_modules` lookups
This is a backport of 4cf8928 and
fa11d48 (#190 and #191) to the 1.x branch.

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

This comment has been minimized.

Copy link
Collaborator

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

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2019

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 keithamus:new-add-isdirectory-use-to-speed-up-node-modules-lookups branch from 7b166f2 to d2816d8 May 15, 2019

@ljharb

ljharb approved these changes May 15, 2019

@ljharb ljharb merged commit 38559e0 into browserify:1.x May 15, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

ljharb added a commit that referenced this pull request May 15, 2019

v1.11.0
 - [New] Add `isDirectory`; use to speed up `node_modules` lookups (#192, #191, #190)
 - [Tests] up to `node` `v12.2`, `v11.15`, `v6.17`

@keithamus keithamus deleted the keithamus:new-add-isdirectory-use-to-speed-up-node-modules-lookups branch May 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.