Node: working directory makes `require.resolve` fail with custom `paths` on v8.x

[FND]

• Version: v8.9.4
• Platform: Darwin 17.4.0
• Subsystem: modules

Invoking require.resolve with the paths option fails if the respective directory does not match the current working directory. This appears to have been fixed in v9.x, but continues to affect LTS versions.

Reduced test case below - I hope these Bash one-liners are readily understandable, otherwise please let me know:

$ cd /tmp

# setup: create sample project with dummy package
$ mkdir -p test_case/node_modules/dummy
$ echo 'console.log("…");' > test_case/node_modules/dummy/index.js

# variant A [FAILS]: working directory does not match `require.resolve.paths`
$ pwd
/tmp
$ node -e 'console.log(require.resolve("dummy/index.js", { paths: ["/tmp/test_case"] }))'
Error: Cannot find module 'dummy/index.js'

# variant B [SUCCEEDS]: working directory and `require.resolve.paths` are identical
$ cd test_case
$ pwd
/tmp/test_case
$ node -e 'console.log(require.resolve("dummy/index.js", { paths: ["/tmp/test_case"] }))'
/tmp/test_case/node_modules/dummy/index.js

Setting process.chdir("/tmp/test_case") before invoking require.resolve does not make any difference.

On Node v9.5.0, both variants work as expected (i.e. just like variant B).

[cjihrig]

If it works in Node 9, you probably are looking for the changes in https://github.com/nodejs/node/commit/8578e81b2b41eac6aea7a812b8e5f35a7362a6df. It looks like it has been backported to the v8.x-staging branch. However, I don't see it in the upcoming 8.10.0 release proposal.

[FND]

Thanks @cjihrig; I'd looked through the changelog, but wasn't sure whether that commit was relevant.

Is there a way I could confirm this fix by locally patching Node? I've extracted the v8.9.4 tarball, but can't find any relevant .js files within. (I not familiar with Node's internals.)

Also, should I pipe up in #18336, requesting inclusion after confirming the above? (I hesitate to just barge in there because I'm equally unfamiliar with Node's community processes.)

[bnoordhuis]

@FND Did you download the binary tarball? You'll need the source tarball, JS files are in lib/.

You can apply the patch by tacking on .patch (or .diff) to the commit URL Colin posted, i.e., https://github.com/nodejs/node/commit/8578e81b2b41eac6aea7a812b8e5f35a7362a6df.patch

[FND]

@bnoordhuis: You're right, thanks - I'd only checked the binary tarball because I had _assumed_ 🤦‍♂️ mere JavaScript files didn't require compiling Node from source.

After applying 8578e81b2b41eac6aea7a812b8e5f35a7362a6df, my test case above passes with the patched v8.9.4 binary - so @cjihrig was indeed correct.

What's the next step for getting this backported for an LTS release?

[bnoordhuis]

Let's ask. @gibfahn Is 8578e81 on track for the next v8.x?

[gibfahn]

Let's ask. @gibfahn Is 8578e81 on track for the next v8.x?

Yes.

[richardlau]

https://github.com/nodejs/node/commit/8578e81b2b41eac6aea7a812b8e5f35a7362a6df was backported to 8.10.0 as https://github.com/nodejs/node/commit/aaca447333.