TypeError: Path must be a string. Received undefined

[simonhaenisch]

Following up on #177 (comment) (not sure whether it's related), 1.9.0 has this code:

resolve/lib/async.js

Lines 47 to 50 in 254bb40

	fs.realpath(absoluteStart, function (realPathErr, realStart) {
	if (realPathErr && realPathErr.code !== 'ENOENT') cb(err);
	else init(realStart);
	});

which throws an error if fs.realpath fails and the error is ENOENT, because then realStart is not defined.

TypeError: Path must be a string. Received undefined

This was already fixed in master in a commit prior to the release 1.9.0 (2321cd4#diff-688ede0ee2da1fa3d89f2fc485f39273), however has not yet been released.

Here is the same part with the fix on master:

resolve/lib/async.js

Lines 58 to 61 in 7c6afab

	fs.realpath(absoluteStart, function (realPathErr, realStart) {
	if (realPathErr && realPathErr.code !== 'ENOENT') cb(err);
	else validateBasedir(realPathErr ? absoluteStart : realStart);
	});

Can we get this fix out any time soon?

[simonhaenisch]

@ljharb could we get this

v1.9.0...simonhaenisch:fix-path-must-be-string

out as patch v1.9.1? I ran the tests, they are all passing.

Without this it's not currently possible to use resolve for non-real paths, which I need for a testing environment with an in-memory (virtual) file system. Sorry, that's actually possible by enabling preserveSymlinks, however that's not the desired solution for me in my case.

[ljharb]

@simonhaenisch it's already in v1.9.0 - see 765b3cb

[simonhaenisch]

It's not in the v1.9.0 tag that was released to npm... see

https://github.com/browserify/resolve/blob/v1.9.0/lib/async.js#L49

[ljharb]

The realpath fix is; the "throw on invalid directory" breaking change is not (and won't be).

[simonhaenisch]

So the problem is:

• I'm not using preserveSysmlinks: true
• I run path.resolve for a directory that doesn't exist
• fs.realpath(absoluteStart, ... has an error
• the error is realPathErr.code = "ENOENT", so the callback doesn't get called
• realStart is undefined
• init(realStart) is called (because of ENOENT)
• res = path.resolve(basedir, x); is called in line 58
• TypeError: Path must be a string. Received undefined (because basedir is realStart which is undefined)

Expected behavior: if it's not a real path, pass the original absoluteStart instead of realStart, i. e. init(realPathErr ? absoluteStart : realStart), and still resolve.

Does that clarify it?

[ljharb]

@simonhaenisch i'm super on board with releasing a v1.9.1 that fixes this for you; is there any chance you could make a PR into v1.9.0 (not master) with a failing test case?

[simonhaenisch]

Please see v1.9.0...simonhaenisch:fix-path-must-be-string, here is the test results without and with the fix.

Before:

# non-existent basedir should not throw when preserveSymlinks is false
path.js:28
    throw new TypeError('Path must be a string. Received ' + inspect(path));
    ^

TypeError: Path must be a string. Received undefined
    at assertPath (path.js:28:11)
    at Object.resolve (path.js:1168:7)
    at init (~/resolve/lib/async.js:58:24)
    at ~/resolve/lib/async.js:49:18
    at gotStat (fs.js:1783:21)
    at FSReqWrap.oncomplete (fs.js:152:21)

After:

# non-existent basedir should not throw when preserveSymlinks is false
ok 1 should be equal
ok 2 should be equal

1..2
# tests 2
# pass  2

# ok

(ran with Node v8.15.0 on macOS)

---

My branch is based of v1.9.0, but I'm not sure how to open a PR against that because it seems I can only open one against a branch but not a tag. Maybe you'll have to git checkout -b v1.9.0-patch v1.9.0 first and push it, so I can open the PR?

I don't have a Windows instance to run the tests on btw.

[simonhaenisch]

@ljharb could you please create a branch based of the v1.9.0 tag and push it to remote, so I can open the PR?

[ljharb]

@simonhaenisch sorry for the delay; done! https://github.com/browserify/resolve/tree/1.9.0-next

[simonhaenisch]

Thanks! PR is open (:

[ljharb]

Merged into the next v1 release (will be v1.10.0); not yet merged into master. When it is, this will close.

[ljharb]

v1.10.0 is released; merged into master with ea7d38d