Skip to content

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

Closed
Closed
require(".\\") doesn't resolve index.js on Windows#18299
Labels
moduleIssues and PRs related to the module subsystem.windowsIssues and PRs related to the Windows platform.

Description

@jdalton
jdalton
opened on Jan 22, 2018

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'

Activity

  1. jdalton commented on Jan 22, 2018

    @jdalton
    jdalton
    on Jan 22, 2018
    MemberAuthor

    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.

  2. bmeck commented on Jan 24, 2018

    @bmeck
    bmeck
    on Jan 24, 2018
    Member

    @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?

  3. joachimvh
    added a commit that references this issue on May 30, 2018

    Replace all backslashes with forward slashes in paths

    c7f9f66
  4. rubensworks
    added a commit that references this issue on May 30, 2018

    Replace all backslashes with forward slashes in paths (#12)

    cc81cf7

  5. Trott commented on Jul 20, 2019

    @Trott
    Trott
    on Jul 20, 2019
    Member

    @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?

  6. Trott commented on Jul 20, 2019

    @Trott
    Trott
    on Jul 20, 2019
    Member

    @jdalton ^^^^

  7. targos
    added
    moduleIssues and PRs related to the module subsystem.
    windowsIssues and PRs related to the Windows platform.
    on Dec 11, 2019

  8. Trott commented on Apr 22, 2020

    @Trott
    Trott
    on Apr 22, 2020
    Member

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

  9. jasnell commented on Jun 25, 2020

    @jasnell
    jasnell
    on Jun 25, 2020
    Member

    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.

  10. jasnell
    closed this as completedon Jun 25, 2020

  11. ljharb commented on Jun 25, 2020

    @ljharb
    ljharb
    on Jun 25, 2020
    SponsorMember

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

  12. jasnell commented on Jun 25, 2020

    @jasnell
    jasnell
    on Jun 25, 2020
    Member

    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

  13. ljharb commented on Jun 25, 2020

    @ljharb
    ljharb
    on Jun 25, 2020
    SponsorMember

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

  14. jasnell
    reopened this on Jun 25, 2020

  15. jasnell commented on Jun 26, 2020

    @jasnell
    jasnell
    on Jun 26, 2020
    Member

    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" ...

  16. pd4d10 commented on May 4, 2021

    @pd4d10
    pd4d10
    on May 4, 2021
    Contributor

    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

  17. Trott commented on May 4, 2021

    @Trott
    Trott
    on May 4, 2021
    Member

    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.

  18. Trott
    closed this as completedon May 4, 2021

  19. Trott commented on May 4, 2021

    @Trott
    Trott
    on May 4, 2021
    Member

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

  20. Trott
    reopened this on May 4, 2021

  21. pd4d10 commented on May 4, 2021

    @pd4d10
    pd4d10
    on May 4, 2021
    Contributor

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

    All cases do not throw errors

  22. Trott
    closed this as completedon May 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    moduleIssues and PRs related to the module subsystem.windowsIssues and PRs related to the Windows platform.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions