Node: should the fact that `this` is bound to module.exports be documented?

[sam-github]

% node -p 'require("./f.js")'
{ fu: 'hi' }
% cat f.js
this.fu = 'hi'

This is not documented, should it be?

cf. https://github.com/nodejs/node/pull/9622#issuecomment-260727547 and earlier comments

[iamchenxin]

test with "babel-plugin-transform-es2015-modules-commonjs": "^6.18.0"
this is set to undefined.

[iamchenxin]

The reason of why this is set to undefined from babel.
why-is-this-being-remapped-to-undefined
Seems this should be documented ?

[mscdex]

IMHO it shouldn't be documented so that users do not start relying on it instead of the proper exports/module.exports.

[iamchenxin]

@mscdex Should it be documented for the difference between strict mode and normal? or documented for prohibiting using this to export. Cause of there is some one use this to export previously.

[JacksonTian]

never be documented.

[sam-github]

I don't like to encourage its use, but on the other hand, it is used, and if you use it by accident, and then want to understand why your code worked the way it does, its nice to find docs that explain that.

Put another way: if we hate this so much we don't want to document it... can we just remove it?

I suspect it will break the world if removed... in which case, we won't remove it, so why not describe it, and also describe how it makes your code node-specific?

[targos]

An option would be to document it as being deprecated, saying that exports should be used instead.

[sam-github]

It should be documented so that if anybody finds code doing it, they can understand what is happening. It should also be doced as deprecated.

Any chance we can remove the feature, or is it harmless to leave in indefinitely? Will a future v8 update be likely to remove the feature for node? Would docs-deprecation be the first step to runtime deprecation?

[TimothyGu]

Any chance we can remove the feature, or is it harmless to leave in indefinitely?

Even though I've never seen this actually used, nor do I encourage it, my fear is that people might be actually using this. And since we don't actually have a _need_ to deprecate this, I'd support maintaining the status quo.

Will a future v8 update be likely to remove the feature for node?

The this behavior is entirely implemented in JS and doesn't depend on any V8 specifics: https://github.com/nodejs/node/blob/51cea054a276c6425e26b25219e67b37207dee3f/lib/module.js#L571. So no, I don't think so.

Note: the upcoming ES module implementation has this === undefined per spec, and since it's a completely different execution mode it doesn't change the behavior discussed here.

[gibfahn]

+1 to documenting as deprecated

[nikniv]

Hey, I can help with this if given some pointers about what needs to be included. :slightly_smiling_face:

Here's what I have so far:

1. Code snippet demonstrating the behaviour
2. Deprecation warning
3. Briefly explain the reason for this code behaviour
4. Recommend using exports instead

Also which section of /docs/api/modules.md file should it go in?

Please let me know. Meanwhile I'll start on this.

Thanks!

[BridgeAR]

@NiveditN deprecations go into the deprecations.md. I think it would be best if you just open a PR as soon as you personally have the feeling it is usable. If there are further improvements necessary they can be done in the open PR.

[nikniv]

Thanks for the info @BridgeAR! I will create the PR as soon as it has something usable.