Skip to content

module: createRequireFromPath documentation is incorrect #23710

Closed
Closed
module: createRequireFromPath documentation is incorrect#23710
Labels
docIssues and PRs related to Node.js documentation.good first issueIssues that are suitable for first-time contributors.help wantedIssues that need assistance from volunteers or PRs that need help to proceed.moduleIssues and PRs related to the module subsystem.

Description

@gillesdemey
gillesdemey
opened on Oct 17, 2018
  • Version: v10.12.0
  • Platform: MacOS 10.14 (Darwin)
  • Subsystem:

The documentation for createRequireFromPath makes it seem like you can pass a directory to the function, but that will never correctly resolve packages.

Using the following directory structure;

src/
└── utils
    └── some-tool.js

The following code will throw a MODULE_NOT_FOUND exception (example from the docs):

const { createRequireFromPath } = require('module');
createRequireFromPath('../src/utils')('some-tool');

However, if we pass a file path we can resolve it just fine.

const { createRequireFromPath } = require('module');
createRequireFromPath('../src/utils/index.js')('some-tool');

Is there a bug here or is the documentation wrong? I'm willing to make a PR to either fix the issue or update the documentation :)

Activity

  1. demurgos commented on Oct 18, 2018

    @demurgos
    demurgos
    on Oct 18, 2018
    Contributor

    I don't believe the documentation is incorrect, but it could be improved.

    Filename to be used to construct the relative require function.

    Only a file can be a module. Your code is resolving from an extension-less file named utils in the src directory. You can't reliably use the path to differentiate directories from files (you can have a directory named utils.js, trailing separators are usually a source of bugs, etc.) and this function can be used with paths that do not exist on the FS.

    It may be worth clarifying that a commonjs module id is the absolute system-dependent path to the corresponding file.

    Edit: I missed the example below: I agree that it using a directory seems to be an error.

  2. targos commented on Oct 18, 2018

    @targos
    targos
    on Oct 18, 2018
    Member

    The example in the documentation seems indeed wrong because it passes a directory to the function.

    /cc @devsnek

  3. guybedford commented on Oct 18, 2018

    @guybedford
    guybedford
    on Oct 18, 2018
    Contributor

    Yes it looks like the example should be changed to include a trailing separator:

    const requireUtil = createRequireFromPath('../src/utils/');

  4. gillesdemey commented on Oct 18, 2018

    @gillesdemey
    gillesdemey
    on Oct 18, 2018
    ContributorAuthor

    @guybedford It works when appending a trailing slash and a ., not when just appending a trailing slash.

    const requireUtil = createRequireFromPath('../src/utils/.');
  5. devsnek
    added
    docIssues and PRs related to Node.js documentation.
    moduleIssues and PRs related to the module subsystem.
    good first issueIssues that are suitable for first-time contributors.
    on Oct 18, 2018

  6. guybedford commented on Oct 18, 2018

    @guybedford
    guybedford
    on Oct 18, 2018
    Contributor

    @gillesdemey thanks for confirming. That sounds incorrect to me. I specially included logic to handle retaining trailing slashes in the pathToFileURL function.

  7. guybedford commented on Oct 18, 2018

    @guybedford
    guybedford
    on Oct 18, 2018
    Contributor

    Ah, I get we're in path resolution and this is how Node path resolution works generally, which is a difference with how URLs behave.

  8. guybedford commented on Oct 18, 2018

    @guybedford
    guybedford
    on Oct 18, 2018
    Contributor

    It would seem sensible to me to specifically add support for a trailing separator in this function though.

  9. gillesdemey commented on Oct 18, 2018

    @gillesdemey
    gillesdemey
    on Oct 18, 2018
    ContributorAuthor

    I can submit a PR to support passing a directory?

    The following change seems to do the trick :)

    const searchPath = filename.endsWith('/') ?
    filename :
    path.dirname(filename);

  m.paths = Module._nodeModulePaths(searchPath);

  10. adeelibr commented on Oct 21, 2018

    @adeelibr
    adeelibr
    on Oct 21, 2018

    Can I pretty please make a PR for this. This can be my first PR to NodeJS.

  11. guybedford commented on Oct 21, 2018

    @guybedford
    guybedford
    on Oct 21, 2018
    Contributor

    I'd be glad to see either PR along these lines for the docs or the trailing separator behaviour.

  12. gillesdemey commented on Oct 22, 2018

    @gillesdemey
    gillesdemey
    on Oct 22, 2018
    ContributorAuthor

    I've created a PR that will allow passing a directory, updating the documentation is still on my todo list 😄

  13. Trott
    added
    help wantedIssues that need assistance from volunteers or PRs that need help to proceed.
    on Nov 19, 2018

  14. doertedev commented on Nov 30, 2018

    @doertedev
    doertedev
    on Nov 30, 2018

    @gillesdemey 's that okay?

  15. creativecomposer commented on Mar 11, 2019

    @creativecomposer
    creativecomposer
    on Mar 11, 2019

    Hi folks, stumbled upon this issue when searching for "good first issue" and "help wanted".

    1. There is a PR to allow passing a directory to createRequireFromPath that is still open.

    2. There is a doc PR that adds the comment // can also be a file to the example assuming that the aforementioned PR is landed.

    If I understand correctly, the assumption made at (2) is incorrect.

    Until the PR at (1) is accepted, the best option I believe is @Trott suggestion at #24763 (comment)

    Shall I go ahead and make the doc change accordingly?

    Just want to make sure my understanding is correct before creating a PR.

  16. guybedford commented on Mar 11, 2019

    @guybedford
    guybedford
    on Mar 11, 2019
    Contributor

    A docs PR to align the docs would be really great and is much needed, thank you @antonyj75 . I'd suggest pushing on the directory PR if you can as well - it could still likely land as-is I think.

  17. creativecomposer commented on Mar 12, 2019

    @creativecomposer
    creativecomposer
    on Mar 12, 2019

    @guybedford thank you for your response.
    Since doc alignment is much needed and is easy, I went ahead and created a PR for that.

    The directory PR seem to be stuck while @gillesdemey added a negative test case.
    I will need a bit more time to figure out what is going on there.

  18. MylesBorins
    added a commit that references this issue on Apr 30, 2019

    module: allow passing a directory to createRequireFromPath

    a734696
  19. MylesBorins
    closed this as completedin e5c8be2on May 2, 2019
  20. targos
    added a commit that references this issue on May 4, 2019

    module: allow passing a directory to createRequireFromPath

    be9a1ec
  21. sjarmak
    added a commit that references this issue on Jul 27, 2026

    module: allow passing a directory to createRequireFromPath

    e5c8be2
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

    docIssues and PRs related to Node.js documentation.good first issueIssues that are suitable for first-time contributors.help wantedIssues that need assistance from volunteers or PRs that need help to proceed.moduleIssues and PRs related to the module subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions