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.

Sign up for GitHub

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

Jump to bottom

Confusing error message when "basedir" is not a directory #154

Closed
gaearon opened this issue Apr 3, 2018 · 1 comment
Closed

Confusing error message when "basedir" is not a directory #154

gaearon opened this issue Apr 3, 2018 · 1 comment
Assignees
@ljharb

Comments

@gaearon
Copy link

gaearon commented Apr 3, 2018

I mistakingly passed a filename as basedir option.
I found the error message quite confusing:

TypeError: Cannot read property 'isFile' of undefined
    at isFile (/Users/gaearon/p/test113/node_modules/resolve/lib/sync.js:12:20)
    at loadAsFileSync (/Users/gaearon/p/test113/node_modules/resolve/lib/sync.js:36:13)
    at loadNodeModulesSync (/Users/gaearon/p/test113/node_modules/resolve/lib/sync.js:75:21)

This is because there is an early exit for "file not found" errors, but for any other errors we fall through to where stat doesn't exist.

Just filing this for future readers.
@ljharb ljharb self-assigned this Apr 4, 2018
@ljharb
Copy link
Member

ljharb commented Apr 4, 2018

Thanks; I've added some tests to ensure that the behavior doesn't change.

In the next major (because changing error messages has been a breaking change for this lib in the past), I'll improve the error message here so that it's explicit about what you did wrong :-)

ljharb added a commit that referenced this issue Apr 4, 2018
@ljharb
[Tests] add some tests for a non-directory basedir
0c18e40 
Related to #154.
ljharb added a commit that referenced this issue Apr 7, 2018
@ljharb
v1.7.0
bdf1210 
 - [New] `resolve.sync`: add `opts.pathFilter` (#146)
 - [Fix] Make loadAsFileSync() work the same as async loadAsFile() (#146)
 - [Fix] support `opts.package` in non-relative lookups (#152)
 - [Docs] fix default “isFile” implementations
 - [Docs] fix options formatting
 - [Refactor] cache default isFile functions at module level
 - [Refactor] use "basedir" instead of "y", because meaningful variable names
 - [Tests] add some tests for a non-directory basedir (#154)
 - [Tests] work around npm SSL issue
 - [Tests] add some tests for browser field
 - [Tests] add node 8 and 9 to appveyor
@ljharb ljharb closed this as completed in 698a3e1 Jun 17, 2018
ljharb added a commit to keithamus/resolve that referenced this issue May 15, 2019
@ljharb
[New] add isDirectory
b502f7c 
Backport of 698a3e1 to 1.x without the breaking change.

See browserify#154.
ljharb added a commit that referenced this issue Dec 23, 2019
@ljharb
v2.0.0-next.0
6f57476 
  - [Breaking] make `preserveSymlinks` false by default (#135)
  - [Breaking] `packageFilter`: bring sync/async into alignment; pass three args: pkg, pkgfile, dir (#171)
  - [Breaking] add `isDirectory`; error early if provided `basedir` is not a directory (#154)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ljharb ljharb

Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ljharb @gaearon