1

I am defining a simple object "Browser" which is allowing to display "previous" and "next" image from a list.

function Browser(image, elements) {
    this.current = 0;
    this.image = image;
    this.elements = elements;
}
Browser.prototype.next = function () {
    if (++this.current >= this.elements.length) {
         this.current = 0;
    }
    return this.show(this.current);
};
Browser.prototype.prev = function () {
    if (--this.current < 0) {
        this.current = this.elements.length - 1;
    }
    return this.show(this.current);
};
Browser.prototype.show = function (current) {
    this.image.src = this.elements[current];
    return false;
};

This code is almost liked by JSlint. But the Google Closure Compiler in "advanced optimization" does not compile it.

It says:

JSC_USED_GLOBAL_THIS: dangerous use of the global this object at line 3 character 0
this.current = 0;
JSC_USED_GLOBAL_THIS: dangerous use of the global this object at line 4 character 0
this.image = image;
JSC_USED_GLOBAL_THIS: dangerous use of the global this object at line 5 character 0
this.elements = elements;

Which tells me that I do not understand javascript oop.

What am I doing wrong?

Toto
  • 2,329
  • 22
  • 40
  • Try removing the `this.` from the constructor of your Browser class, does it work now? – Dunhamzzz Feb 08 '13 at 17:31
  • This is a warning that if the Closure compiler flattens the namespace to shrink the code size, `this` may no longer refer to the correct object. Toto, is SO post http://stackoverflow.com/questions/5301373/closure-compiler-warning-dangerous-use-of-the-global-this-object a duplicate of your question? – dgvid Feb 08 '13 at 17:32
  • @dgvid -- with all respect, no. Even without Closure, if some future programmer calls `Browser()` (instead of `new Browser()` as he should), `current`, `image`, and `elements` will be written, as you were warned, into global namespace. JavaScript is a sharp tool; be careful. – Michael Lorton Feb 08 '13 at 17:41
  • 1
    @Malvolio -- with all respect, yes. Read the link that dgvid posted, the second answer. It explains what dgvid incompletely commented with. You're still right about not using `new` though – Ian Feb 08 '13 at 17:51

2 Answers2

6

JSC_USED_GLOBAL_THIS: dangerous use of the global this object.

This warning means that you use the keyword this outside of prototype function or a constructor function. For example, the following code will produce this warning:

// Produces JSC_USED_GLOBAL_THIS warning:
this.foo = 1;

In this case this actually refers to the global this object. For a standard web page, the global object is the same thing as the window object. If you get this warning, make sure that you really intended to refer to the global object.

Note that the compiler only recognizes a function as a constructor if it is annotated with the @constructor JSDoc annotation, like this:

/**
 * @constructor
 */
function MyFunction() {
  this.foo = 1;
}

https://developers.google.com/closure/compiler/docs/error-ref

Peter
  • 16,453
  • 8
  • 51
  • 77
4

From the docs:

Note that the compiler only recognizes a function as a constructor if it is annotated with the @constructor JSDoc annotation, like this:

/**
 * @constructor
 */
function MyFunction() {
  this.foo = 1;
}
Michael Lorton
  • 43,060
  • 26
  • 103
  • 144