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.

Sign up for GitHub

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

Jump to bottom

module.paths is undocumented #14371

Closed
ScottFreeCode opened this issue Jul 19, 2017 · 8 comments
Closed

module.paths is undocumented #14371

ScottFreeCode opened this issue Jul 19, 2017 · 8 comments
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. module Issues and PRs related to the module subsystem. question Issues that look for answers.

Comments

@ScottFreeCode
Copy link

  • Version: 8.1.4
  • Platform: nodejs.org documentation
  • Subsystem: module

Node exposes the apparently non-private variable module.paths, but there is no documentation about it on nodejs.org, not even to say that it is really private despite the naming convention (I don't know if this is the case).
@mscdex mscdex added module Issues and PRs related to the module subsystem. question Issues that look for answers. labels Jul 19, 2017
@vsemozhetbyt vsemozhetbyt added the doc Issues and PRs related to the documentations. label Jul 19, 2017
@ScottFreeCode ScottFreeCode mentioned this issue Jul 19, 2017
Merged
@gibfahn
Copy link
Member

gibfahn commented Jul 22, 2017

Looking at git blame it looks like it was added in 4651348 (by @isaacs in 2011).

I suspect this is an oversight. I'll mark this as a documentation request, and if people have strong opinions about not documenting it they can voice them here or in the PR.

@gibfahn gibfahn added the good first issue Issues that are suitable for first-time contributors. label Jul 22, 2017
@gibfahn
Copy link
Member

gibfahn commented Jul 22, 2017

For anyone looking to take this on, the first thing to read/follow is CONTRIBUTING.md, there's also a Style Guide that might be useful, although make lint lints the docs now, and following the style in that file is usually a good way to go.

The place to add module.paths documentation is here (in doc/api/modules.md).

@atever
Copy link
Contributor

atever commented Jul 22, 2017

I'd like to give it a try.

@gibfahn
Copy link
Member

gibfahn commented Jul 23, 2017

I'd like to give it a try.

Go for it, if you have any problems just ask here. Once you have something just raise a PR, if it's not finished put WIP in the title. That way people can give you feedback on your proposed change.

atever added a commit to atever/node that referenced this issue Jul 23, 2017
@atever
doc: add docs for module.paths
40bd5cf 
- Refs: nodejs#14371 (comment)
atever added a commit to atever/node that referenced this issue Jul 23, 2017
@atever
doc: add docs for module.paths
b09fd21 
- Refs: nodejs#14371 (comment)
@atever atever mentioned this issue Jul 23, 2017
Closed
2 tasks
atever added a commit to atever/node that referenced this issue Jul 23, 2017
@atever
doc: correct the docs for module.paths
9adbe66 
Change the `module.paths` type to `string[]`.

According to alphabetical order, Move the `module.paths` after `module.parent`.

Refs: nodejs#14371 (comment)
@atever
Copy link
Contributor

atever commented Jul 23, 2017 •
edited
Loading

@gibfahn thx, there one question that the added Version is correct？

refack pushed a commit that referenced this issue Jul 25, 2017
@atever @refack
doc: add docs for module.paths
9ab3172 
PR-URL: #14435
Refs: #14371 (comment)
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
addaleax pushed a commit that referenced this issue Jul 27, 2017
@atever @addaleax
doc: add docs for module.paths
ab9bc81 
PR-URL: #14435
Refs: #14371 (comment)
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@JaySeeCode
Copy link

Has this issue been resolved?

@TimothyGu
Copy link
Member

TimothyGu commented Jul 31, 2017 •
edited by refack
Loading

@JaySeeCode Yes, in #14435. Sorry, I'll close this issue now.

@refack
Copy link
Contributor

refack commented Jul 31, 2017

@JaySeeCode #14435 is a good start, but if you have the time to research it a little bit, try expanding the description, maybe also add a reference to the resolve algorithm. The other fields could use some love also - https://github.com/nodejs/node/blob/master/doc/api/modules.md#modulepaths

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. module Issues and PRs related to the module subsystem. question Issues that look for answers.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@mscdex @refack @TimothyGu @vsemozhetbyt @atever @gibfahn @ScottFreeCode @JaySeeCode