doc: improve description of module `exports` #9622

sam-github · 2016-11-15T18:36:17Z

Checklist

documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

doc

Description of change

doc: improve description of module exports

- 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.
- 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

Replace #9552

Fix #9552 (comment)

iamchenxin · 2016-11-15T18:39:06Z

What about this, i am not sure if i missed something. i can not find the definition of this in old doc.

sam-github · 2016-11-15T18:41:59Z

Above PRs/issues address specific comments you have made.

Do you have more, or can we close this PR?

iamchenxin · 2016-11-15T18:43:40Z

Yes, seems this will be solved in 9623

iamchenxin · 2016-11-15T18:47:07Z

And seems document this will make a break change. Cause of some third part tools set different values for this. I should check the babel-plugin again, to test if it is still break this.

iamchenxin · 2016-11-15T19:09:01Z

Thanks for your patience again, open an issue in babel for asking this.
I gotta sleep, have a good day.

jasnell · 2016-11-18T16:01:21Z

doc/api/modules.md

-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.


Please avoid the use of informal pronouns such as you, your and us in the docs :-)

jasnell · 2016-11-18T16:02:37Z

doc/api/modules.md

+
+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`:


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."

sam-github · 2016-11-18T17:15:52Z

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.

jasnell · 2016-11-18T17:21:59Z

doc/api/modules.md

+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


these should be wrapped in an explicit code block using the three backticks...

Done, thanks, sorry for the churn, I'll be up to speed on current conventions soon.

Qard · 2016-11-19T05:24:11Z

LGTM

lpinca · 2016-11-19T20:55:41Z

doc/api/modules.md

+```
+
+When the `module.exports` property is being completely replaced by a new
+object, it is common to also reassign `exports`, for example:


Is this really widely used or a good practice?

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.

@lpinca: https://github.com/search?p=2&q=%22module.exports+%3D+exports%22&type=Code&utf8=%E2%9C%93

sam-github · 2016-11-21T17:29:31Z

@jasnell can you confirm I made your requested changes?

sam-github · 2016-11-23T15:14:43Z

@jasnell I don't want to merge while your review shows "requested changes" - I made the changes, can you confirm/LGTM?

jasnell

Lgtm! Sorry for the delay!

sam-github · 2016-11-29T15:46:15Z

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 <luigipinca@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: James M Snell <jasnell@gmail.com>

- 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 <luigipinca@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: James M Snell <jasnell@gmail.com>

sam-github added the doc Issues and PRs related to the documentations. label Nov 15, 2016

nodejs-github-bot added the module Issues and PRs related to the module subsystem. label Nov 15, 2016

sam-github force-pushed the improve-exports-alias-docs branch from 28b6e92 to 84b18c2 Compare November 15, 2016 18:36

sam-github mentioned this pull request Nov 15, 2016

doc: add more detailed illustration for module.exports and exports. #9552

Closed

2 tasks

sam-github mentioned this pull request Nov 15, 2016

should the fact that this is bound to module.exports be documented? #9623

Closed

jasnell requested changes Nov 18, 2016

View reviewed changes

sam-github force-pushed the improve-exports-alias-docs branch 3 times, most recently from d2e5243 to a5ee1d1 Compare November 18, 2016 17:10

jasnell reviewed Nov 18, 2016

View reviewed changes

sam-github force-pushed the improve-exports-alias-docs branch 2 times, most recently from 0233696 to 472b2df Compare November 18, 2016 17:59

lpinca reviewed Nov 19, 2016

View reviewed changes

lpinca approved these changes Nov 19, 2016

View reviewed changes

sam-github force-pushed the improve-exports-alias-docs branch from 472b2df to e8f3290 Compare November 21, 2016 17:30

sam-github mentioned this pull request Nov 29, 2016

doc: doc/api/modules.md needs a better explanation of exports #9840

Closed

jasnell approved these changes Nov 29, 2016

View reviewed changes

sam-github force-pushed the improve-exports-alias-docs branch from e8f3290 to 5887c2b Compare November 29, 2016 15:48

sam-github merged commit 5887c2b into nodejs:master Nov 29, 2016

Fishrock123 mentioned this pull request Dec 5, 2016

v7.2.1 proposal #10127

Merged

2 tasks

MylesBorins added land-on-v4.x labels Dec 20, 2016

This was referenced Dec 21, 2016

V6.9.3 proposal #10394

Merged

V4.7.1 proposal #10395

Merged

sam-github deleted the improve-exports-alias-docs branch April 17, 2017 20:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: improve description of module `exports` #9622

doc: improve description of module `exports` #9622

sam-github commented Nov 15, 2016 •

edited

Loading

iamchenxin commented Nov 15, 2016

sam-github commented Nov 15, 2016

iamchenxin commented Nov 15, 2016

iamchenxin commented Nov 15, 2016

iamchenxin commented Nov 15, 2016

jasnell Nov 18, 2016

jasnell Nov 18, 2016

sam-github commented Nov 18, 2016

jasnell Nov 18, 2016

sam-github Nov 18, 2016

Qard commented Nov 19, 2016

lpinca Nov 19, 2016

Qard Nov 20, 2016

sam-github Nov 21, 2016

sam-github commented Nov 21, 2016

sam-github commented Nov 23, 2016

jasnell left a comment

sam-github commented Nov 29, 2016

doc: improve description of module exports #9622

doc: improve description of module exports #9622

Conversation

sam-github commented Nov 15, 2016 • edited Loading

Checklist

Affected core subsystem(s)

Description of change

iamchenxin commented Nov 15, 2016

sam-github commented Nov 15, 2016

iamchenxin commented Nov 15, 2016

iamchenxin commented Nov 15, 2016

iamchenxin commented Nov 15, 2016

jasnell Nov 18, 2016

Choose a reason for hiding this comment

jasnell Nov 18, 2016

Choose a reason for hiding this comment

sam-github commented Nov 18, 2016

jasnell Nov 18, 2016

Choose a reason for hiding this comment

sam-github Nov 18, 2016

Choose a reason for hiding this comment

Qard commented Nov 19, 2016

lpinca Nov 19, 2016

Choose a reason for hiding this comment

Qard Nov 20, 2016

Choose a reason for hiding this comment

sam-github Nov 21, 2016

Choose a reason for hiding this comment

sam-github commented Nov 21, 2016

sam-github commented Nov 23, 2016

jasnell left a comment

Choose a reason for hiding this comment

sam-github commented Nov 29, 2016

doc: improve description of module `exports` #9622

doc: improve description of module `exports` #9622

sam-github commented Nov 15, 2016 •

edited

Loading