doc: improve module.builtinModules documentation #17712
Conversation
nodejs-github-bot
added
doc
doc/api/modules.md
Outdated
| @@ -841,6 +841,14 @@ added: v9.3.0 | |||
| A list of the names of all modules provided by Node.js. Can be used to verify | |||
| if a module is maintained by a third-party module or not. | |||
|
|
|||
| Note that `module` in this context isn't the same object that's | |||
| available by default inside file modules. To access it you need to | |||
There was a problem hiding this comment.
Please avoid using 'you' in the docs
doc/api/modules.md
Outdated
| @@ -841,6 +841,14 @@ added: v9.3.0 | |||
| A list of the names of all modules provided by Node.js. Can be used to verify | |||
| if a module is maintained by a third-party module or not. | |||
|
|
|||
| Note that `module` in this context isn't the same object that's | |||
| available by default inside file modules. To access it you need to | |||
There was a problem hiding this comment.
Instead of saying "available by default inside file modules" maybe say "provided by the module wrapper" and you can link to the module wrapper section.
doc/api/modules.md
Outdated
|
|
||
| ```js | ||
| const builtin = require('module').builtinModules; | ||
| ``` |
There was a problem hiding this comment.
That part is somewhat explained by the brief Accessed via 'require("module")' above.
There was a problem hiding this comment.
True, but to be honest I didn't see this my self. I needed to ask an adult for help. So I think it's worth showing an extra time. But maybe it's just me 😅
| @@ -841,11 +841,19 @@ added: v9.3.0 | |||
| A list of the names of all modules provided by Node.js. Can be used to verify | |||
| if a module is maintained by a third-party module or not. | |||
|
|
|||
| Note that `module` in this context isn't the same object that's provided | |||
There was a problem hiding this comment.
Should this note be moved to below the example for accessing builtinModules? Its placement here makes it in To access it (the next sentence) seem like it refers to the module wrapper which is kind of the opposite of the sentences intention, I think?
There was a problem hiding this comment.
I was actually referring to the module mentioned in the headline of this section just above. But I can see that this can be a bit confusing. Not sure if moving it below the code example helps much unless I also change the code example to assign the required module to a constant like I did in the alternative code example in the PR description. What do you think?
There was a problem hiding this comment.
This is why I like to refer to this module with an uppercase M as it makes it much easier to distinguish between the two
doc/api/modules.md
Outdated
| @@ -841,11 +841,19 @@ added: v9.3.0 | |||
| A list of the names of all modules provided by Node.js. Can be used to verify | |||
| if a module is maintained by a third-party module or not. | |||
|
|
|||
| Note that `module` in this context isn't the same object that's provided | |||
| by the [module wrapper][]. To access it require the `Module` module: | |||
There was a problem hiding this comment.
I don't believe we typically capitalize module names so I suspect this should be the `module` module?
There was a problem hiding this comment.
Regardless, please insert a comma in the last sentence between it and require: To access it, require the...
There was a problem hiding this comment.
Whoops, ignore my comment on casing (module vs. Module) since, you know, that's the whole point... 🙃 Sorry.
|
I left some comments/nits, but nothing that I would consider a blocking objection. |
|
Landing... |
|
Thank you for your contribution, landed in f4ab204. |
PR-URL: nodejs#17712 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #17712 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #17712 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #17712 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: James M Snell <[email protected]>
In this PR I've kept the original casing of the object
module, but I think it might be easier to understand if we changed the entire section to use the uppercase version:Module. This have two benefits:moduleobject that's available by default to all file modulesThat change would instead look like this:
Checklist
Affected core subsystem(s)