require(".\\") doesn't resolve index.js on Windows

[jdalton]

While looking at #15015 (comment) I noticed that the trailing slash check in _findPath was only keying off of a forward slash and not the backslash that Windows allows.

You can repro this by simply doing the following in a directory with an index.js

require(".\\") // throws
require("./") // will find the index.js

Other places in Node account for the backslash in Windows paths so this looks like an oversight.

Update:

It looks like if it's a two dot relative path to an index.js then it does work.

require("..\\") // work
require("..\\.") // works
require("../") // works
require("../.") // works

Also this

require("./..") // works
require(".\\..") // throws

Notes:

It looks like path.resolve handles these cases fine so it can be excluded from the problem.

path.resolve(".\\") // 'C:\\projects\\bar\\foo'
path.resolve(".\\..") // 'C:\\projects\\bar'

[jdalton]

It looks like part of the issue is in Module._resolveLookupPaths because it's only looking for a forward slash

node/lib/module.js

Line 365 in a3555d0

request.charCodeAt(1) !== 47/*/*/)) {

Module._resolveLookupPaths("./", null)
[ './',
  [ '.',
    'C:\\projects\\bar\\foo\\node_modules',
    'C:\\projects\\bar\\node_modules',
    'C:\\projects\\node_modules',
    'C:\\node_modules',
    'C:\\Users\\jdalton\\.node_modules',
    'C:\\Users\\jdalton\\.node_libraries',
    'C:\\Program Files\\nodejs\\lib\\node' ] ]

but is snipped:

> Module._resolveLookupPaths(".\\", null)
[ '.\\',
  [ 'C:\\Users\\jdalton\\.node_modules',
    'C:\\Users\\jdalton\\.node_libraries',
    'C:\\Program Files\\nodejs\\lib\\node' ] ]

⚠️ Once Module._resolveLookupPaths is fixed though it'll run into issues with #15015.

[bmeck]

@jdalton I think we could probably have windows normalize \\ to / in all cases [like the URL spec does], is there a case where \\ doesn't act like / in the existing usage of paths on win32?

[Trott]

@jdalton I think we could probably have windows normalize \\ to / in all cases [like the URL spec does], is there a case where \\ doesn't act like / in the existing usage of paths on win32?

I'm curious about the answer to @bmeck's question here, and also if this is still an issue?

[Trott]

@jdalton ^^^^

[Trott]

@nodejs/modules-active-members @jdalton Should this be closed? Or is this an issue that should be addressed?

[jasnell]

There's been no further action on this. Closing, but given that it's not fully resolved, I'm putting this on the Futures project board so that it does not get lost.

[ljharb]

@jasnell why would an inactive issue that's still a problem be closed?

[jasnell]

An unresolved issue that no one ever looks at isn't useful either. These can always be reopened if someone intends to pick it up. Also, after I'm done taking a triage pass at all these stale old issues I'll be compiling a list of outstanding issues for each subsystem

[ljharb]

How can anyone ever decide to pick it up if it's closed?

[jasnell]

I've reopened but as I said, "I'm putting this on the Futures project board so that it does not get lost." and "after I'm done taking a triage pass at all these stale old issues I'll be compiling a list of outstanding issues for each subsystem" ...

[pd4d10]

Can confirm this issue has been fixed with all these paths:

require(".\\") // throws
require("./") // will find the index.js

require("..\\") // work
require("..\\.") // works
require("../") // works
require("../.") // works

require("./..") // works
require(".\\..") // throws

Tested versions:
v12.22.1
v14.16.1
v15.14.0
v16.0.0

[Trott]

As this appears to be fixed on all supported versions, I'm going to close it. Of course, if that's wrong and it's still an issue somewhere, please comment or re-open.

[Trott]

Oh, wait, no, there's still one problem left, right? require(".\\..") // throws

[pd4d10]

Nope, it's just quoted from the original post.

All cases do not throw errors