-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: improve description of module exports
#9622
doc: improve description of module exports
#9622
Conversation
28b6e92
to
84b18c2
Compare
What about |
Above PRs/issues address specific comments you have made. Do you have more, or can we close this PR? |
Yes, seems |
And seems document this will make a break change. Cause of some third part tools set different values for |
Thanks for your patience again, open an issue in babel for asking |
to `module.exports`. As with any variable, if you assign a new value to it, it | ||
is no longer bound to the previous value. | ||
The `exports` variable that is available within a module starts as if | ||
`module.exports` had been assigned to it before your module was evaluated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid the use of informal pronouns such as you
, your
and us
in the docs :-)
|
||
It allows a shortcut, so that `module.exports.f = ...` can be written more | ||
succinctly as `exports.f = ...`. However, be aware that like any variable, if | ||
you assign a new value to it, it is no longer bound to `module.exports`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, this last sentence is better written as: "Note, however, assigning a new value to exports
causes it to no longer be bound to module.exports
."
d2e5243
to
a5ee1d1
Compare
Thanks @jasnell, I removed the colloquialisms, and also fixed another bug in the require pseudo-code. Personally, I would remove the pseudo code in preference for the textual description and example, but maybe some other time, so it doesn't slow this PR. |
succinctly as `exports.f = ...`. However, be aware that like any variable, if | ||
a new value is assigned to `exports`, it is no longer bound to `module.exports`: | ||
|
||
module.exports.hello = true; // Exported from require of module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should be wrapped in an explicit code block using the three backticks...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks, sorry for the churn, I'll be up to speed on current conventions soon.
0233696
to
472b2df
Compare
LGTM |
``` | ||
|
||
When the `module.exports` property is being completely replaced by a new | ||
object, it is common to also reassign `exports`, for example: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really widely used or a good practice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't say it's universal, but I see it use fairly often as a defensive measure. I'd consider it a relatively good practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jasnell can you confirm I made your requested changes? |
472b2df
to
e8f3290
Compare
@jasnell I don't want to merge while your review shows "requested changes" - I made the changes, can you confirm/LGTM? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm! Sorry for the delay!
Thanks! |
- Do not use word alias, it isn't well defined - Fix return value of require() example, which confusingly was not the exported API as it should have been. - Fix the require() example, which claimed that the module exported `0`, when it exports `some_func`. - Describe best practice in keeping exports and module.exports bound together. - Describe why exports exists - Remove reference to magic, which is also not well defined PR-URL: nodejs#9622 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: James M Snell <[email protected]>
e8f3290
to
5887c2b
Compare
- Do not use word alias, it isn't well defined - Fix return value of require() example, which confusingly was not the exported API as it should have been. - Fix the require() example, which claimed that the module exported `0`, when it exports `some_func`. - Describe best practice in keeping exports and module.exports bound together. - Describe why exports exists - Remove reference to magic, which is also not well defined PR-URL: #9622 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: James M Snell <[email protected]>
- Do not use word alias, it isn't well defined - Fix return value of require() example, which confusingly was not the exported API as it should have been. - Fix the require() example, which claimed that the module exported `0`, when it exports `some_func`. - Describe best practice in keeping exports and module.exports bound together. - Describe why exports exists - Remove reference to magic, which is also not well defined PR-URL: #9622 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: James M Snell <[email protected]>
- Do not use word alias, it isn't well defined - Fix return value of require() example, which confusingly was not the exported API as it should have been. - Fix the require() example, which claimed that the module exported `0`, when it exports `some_func`. - Describe best practice in keeping exports and module.exports bound together. - Describe why exports exists - Remove reference to magic, which is also not well defined PR-URL: #9622 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: James M Snell <[email protected]>
- Do not use word alias, it isn't well defined - Fix return value of require() example, which confusingly was not the exported API as it should have been. - Fix the require() example, which claimed that the module exported `0`, when it exports `some_func`. - Describe best practice in keeping exports and module.exports bound together. - Describe why exports exists - Remove reference to magic, which is also not well defined PR-URL: #9622 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: James M Snell <[email protected]>
- Do not use word alias, it isn't well defined - Fix return value of require() example, which confusingly was not the exported API as it should have been. - Fix the require() example, which claimed that the module exported `0`, when it exports `some_func`. - Describe best practice in keeping exports and module.exports bound together. - Describe why exports exists - Remove reference to magic, which is also not well defined PR-URL: #9622 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: James M Snell <[email protected]>
Checklist
Affected core subsystem(s)
doc
Description of change
doc: improve description of module
exports
Replace #9552
Fix #9552 (comment)