Use `window` instead of `this` in closures to improve obfuscation. #2413

Open
ghost opened this Issue Aug 21, 2012 · 7 comments

5 participants

@ghost

I don't understand why the global object is still referenced by using the keyword this inside the closures since it won't be obfuscated/minimized. As an example, the following...

(function(){

    this.MooTools = { /*! ... */ };

    var typeOf = this.typeOf = function(item){ /*! ... */ };

})();

... will be minimized as ...

(function(){this.MooTools={/*! ... */};var a=this.typeOf=function(b){/*! ... */};})();

... while the now following ...

!function(window){

    window.MooTools = { /*! ... */ };

    var typeOf = window.typeOf = function(item){ /*! ... */ };

}(this);

... will be minimized this way:

!function(a){a.MooTools={/*! ... */};var b=a.typeOf=function(c){/*! ... */};}(this);

(Note: Negating the self invoking function [unless there is no returning value needed] saves the parentheses.)

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@ibolmo ibolmo modified the milestone: 1.5.1, 1.6 Mar 3, 2014
@ibolmo ibolmo added the enhancement label Mar 3, 2014
@SergioCrisostomo
MooTools member

Tested this idea on a branch in my repo and then minified it with grunt-uglify.
The resulting code passed the specs and looks like

!function(a){a.MooTools={version:"1.5.1-dev",build:"%build%"};var b=a.typeOf=function(a){if(null==a)return"null"; etc.....

The original minified file (before this change) was 83Kb, after the change was also 83Kb, just some bytes difference.

I don't say this is a bad idea but got the idea that code source is already well written and that the occurrences of the this word is low in our code.

Should I send a PR with this changes or should we close this issue?

@kentaromiura
MooTools member

I don't like the idea of using window since it's the global object only in browsers, since we provide a server build it will be weird, we should use global instead.

@arian
MooTools member

Also, gzip. It's not that big of a deal iiuc.

@arian arian closed this Jul 6, 2014
@ibolmo
MooTools member
@gonchuki

I actually like the improved readability with explicitly stating what this points to, and as @kentaromiura says it should be using global to keep it generic.

@kentaromiura
MooTools member

One more advantage of using global is that is a step forward cjs, since by exporting to global it will work as a cjs pseudo module in https://github.com/mootools/wrapup for example.

@kentaromiura kentaromiura reopened this Jul 7, 2014
@ghost

It was just my intention to state that the reference of this could also be passed as an argument into the closure. It was not about how to name the argument. window was just an obvious guess. I agree that global would make more sense since there is a server build as well.

@SergioCrisostomo SergioCrisostomo modified the milestone: 1.5.2, 1.5.1 Jul 8, 2014
@ibolmo ibolmo modified the milestone: 1.6.0, 1.5.2 Nov 11, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment