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

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

This is not documented, should it be?

cf. #9622 (comment) and earlier comments

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

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

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

@mscdex

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

never be documented.

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?

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

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?

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:

node/lib/module.js

Line 571 in 51cea05

var result = compiledWrapper.apply(this.exports, args);

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

+1 to documenting as deprecated

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

Here's what I have so far:

Code snippet demonstrating the behaviour
Deprecation warning
Briefly explain the reason for this code behaviour
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!

@niveditn

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

@BridgeAR

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

doc: deprecate top-level `this`

Navigation Menu

Repository navigation

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

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Activity

iamchenxin commented on Nov 15, 2016

iamchenxin commented on Nov 15, 2016

mscdex commented on Nov 15, 2016

iamchenxin commented on Nov 15, 2016

JacksonTian commented on Nov 16, 2016

sam-github commented on Nov 16, 2016

targos commented on Jan 8, 2017

sam-github commented on Jan 9, 2017

TimothyGu commented on Mar 6, 2017

gibfahn commented on Mar 6, 2017

niveditn commented on Aug 11, 2017

BridgeAR commented on Sep 23, 2017

niveditn commented on Sep 25, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

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