Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc fix: align code example for module.createRequireFromPath() with actual code behavior. #26606

Closed

Conversation

creativecomposer
Copy link

module.createRequireFromPath() takes a filename as argument.
But the accompanying code example in the documentation makes it seem
like it can take a directory argument as well.
However, if a directory is passed as argument instead of a filename,
then the function does not work as expected.

Therefore, the fix is to make explicit in the code example that
only filenames could be passed to module.createRequireFromPath()
and not directory names.

Fixes: #23710
Refs: #24763 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. module Issues and PRs related to the module subsystem. labels Mar 12, 2019
module.createRequireFromPath() takes a filename as argument.
But the accompanying code example in the documentation makes it seem
like it can take a directory argument as well.
However, if a directory is passed as argument instead of a filename,
then the function does not work as expected.

Therefore, the fix is to make explicit in the code example that
only filenames could be passed to module.createRequireFromPath()
and not directory names.

Fixes: nodejs#23710
Refs: nodejs#24763 (comment)
@BridgeAR
Copy link
Member

@Antony2025 thanks a lot for the PR! Since there is an open PR that pretty much does the same as your, I would rather get that over the finish line instead. I hope that is OK?

@creativecomposer
Copy link
Author

@BridgeAR remember the other open PR assumes that the directory taking PR is already landed.
But this assumption is not true.

Therefore, the other PR must be changed so that it reflects the current code behavior: module.createRequireFromPath() works fine only when a file name is passed.

See @Trott comment there: #24763 (comment)

@creativecomposer
Copy link
Author

Hi @BridgeAR and @guybedford ,
Since you want to go ahead with the other doc PR, shall I close this one?

Quite frankly, I do not see any response from the other PR author for @Trott review comment, therefore I would much rather prefer my PR - which is as per @Trott comment - to be merged.

Anyway, up to you guys to decide.

@creativecomposer
Copy link
Author

Closing this PR, because there is another open PR to address the issue.

@creativecomposer creativecomposer deleted the 23710-doc-fix branch March 20, 2019 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. module Issues and PRs related to the module subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

module: createRequireFromPath documentation is incorrect
3 participants