Strict Violation using this keyword and revealing module pattern

Question

Having trouble getting the following to pass jslint/jshint

/*jshint strict: true */
var myModule = (function() {
    "use strict";

    var privVar = true,
        pubVar = false;

    function privFn() {
        return this.test; // -> Strict violation.
    }

    function pubFn() {
        this.test = 'public'; // -> Strict violation.
        privFn.call(this); // -> Strict violation.
    }

    return {
        pubVar: pubVar,
        pubFn: pubFn
    };

}());

myModule.pubFn();

I understand it's being caused by the use of this in a function declaration, but I read something Crockford wrote and he said the violation is meant to prevent global variable pollution - but the only global variable here is the one I'm explicitly defining... myModule. Everything else is held in the immediate function scope, and I should be able to use this to refer to the module.

Any ideas how I can get this pattern to pass?

Update: if I use a function expression instead of a declaration, this seems to work, ie

var pubFn = function () { ...

I'm not a fan of this format though, prefer to have the function name and named params closer and the declaration looks/feels cleaner. I honestly don't see why this is throwing the violation - there's no reason for it in this pattern.

Bauske

Sounds like one of those cases where you can just ignore jslint's complaints. As an aside, how does the pubVar work when accessed as myModule.pubVar? It doesn't really give code outside the model access to the variable in the module does it? I would've thought that if you want to actually get/set the current value of the module's variable you'd need getter and setter functions. – Icy

you're right, I thought it looked a bit funny. If you tried myModule.pubVar = true you would just rewrite the property on the object. The internal pubVar would remain as false – Bauske

Re your update: I prefer var foo = function(){} to function foo(){} because it helps avoid hosting problems. Also, I like that it does make the functions look more like other vars, since in JS there is no difference between a function or any other value--they're first class. Personal preference, I know--but I thought I'd throw out some thoughts. – Scabious

Pseudo-offtopic: If you still want to use this pattern, which I personally do not like... you can use the "crockford" variation, i.e. type directly the public interface in the return object. It is even shorter and easier to mantain. Anyway, using closures for private members is not worth it. – Crissie

@ikaros45 "Anyway, using closures for private members is not worth it." I'd like to know why – Sublet

@Mirko well, it boils down to the fact that they are not private variables that belong to an object. They are variables that can be accessed (supposedly) only by a given object. When you start to work with delegation chains, it turns out to be a mess. Here an somehow complete answer I made: #19949198 It is also worth reading Eisailija's answer. – Crissie

Answer 1

JSHint has an option called validthis, which:

[...] suppresses warnings about possible strict violations when the code is running in strict mode and you use this in a non-constructor function [...], when you are positive that your use of this is valid in strict mode.

Use it in the function that JSHint is complaining about, which in your case, would look like this:

function privFn() {
    /*jshint validthis: true */
    return this.test; // -> No Strict violation!
}

function pubFn() {
    /*jshint validthis: true */
    this.test = 'public'; // -> No Strict violation!
    privFn.call(this); // -> No Strict violation!
}

It might seem like a pain to have to specify that in each function where it applies, but if you set the option at the top of your module function, you may hide genuine strict mode violations from yourself.

Teat

Answer 2

The real problem here is that if you call privFn from within the module context (from within the IIFE), this will be undefined when in strict mode; window if not in strict mode. Alas, the function would fail if called from within the IIFE.

This is because the functions have no owner (object) when called from within an IIFE, whereas the returned module object is the owner of the functions when they are called from outside the IIFE context, e.g. this === myModule when calling myModule.pubFn().

Both strict mode and JSHint/JSLint are trying to help you and you should never just ignore the errors/warnings generated by them, but instead figure out why they are warning you.

If you are 100 percent sure that privFn, pubFn, etc. will not be called anywhere but outside your module, just put in the comment /*jshint validthis: true */ in any functions that generate a warning. Alternatively, one comment in the IIFE will prevent JSHint from generating this error on any function inside the module.

---

One of the many possible solution

Store the scope of this (in self in this example) to refer explicitly to the module. This will show and ensure your intent.

/*jshint strict: true */
var myModule = (function() {
    "use strict";

    var privVar = true,
        pubVar = false,
        self = this;

    function privFn() {
        return self.test;
    }

    function pubFn() {
        self.test = 'public';
        //privFn.call(this); // Will have no effect, as `privFn` does not reference `this`
        privFn();
    }

    return {
        pubVar: pubVar,
        pubFn: pubFn
    };
}());

myModule.pubFn();

Uxoricide

Totally agree with your answer just want to add one more thing. If you use 'this' inside a function start with capital letter it'll not give you any warning. Because function start with capital letter means you gonna use it as a constructor with new keyword. You allowed to use this in constructor even in strict mode :) – Caput

I have the same issue for an array map(callback, context) callback which is used in different places. "this" has in this case a perfectly valid usage as it is provided by the second argument of map(). It is by the way also true for other array methods such as some(), every(), reduce(), reduceRight(). I think JSHint & JSLint could check that such function is always used as an array method callback with a provided context – Ripplet

Answer 3

Unfortunately, this is the intended error for this setup since jslint/jshint don't know the function declared in global context is to be later used as an object method.

Bauske

But those functions are not declared in global code. They're nested inside your function expression (which you use to wrap your module's code). – Beaver