Ticket #5404 (closed bug: fixed)

Opened 5 years ago

Last modified 2 years ago

remove uses of 'var self = this;'

Reported by: rdworth Owned by:
Priority: minor Milestone: 1.9.0
Component: [meta] ui.dev Version: 1.8
Keywords: Cc:
Blocking: Blocked by:

Description (last modified by scott.gonzalez) (diff)

DO NOT TRY TO FIX THIS UNTIL RIGHT BEFORE 1.9

Fixing this any sooner will cause merge conflicts for pull requests


var self = this;

is used in many plugins but not recommended best practice because if self isn't defined in a scope it will reference window.self

 http://www.javascriptkata.com/2007/05/14/how-to-use-the-self-with-object-oriented-javascript-and-closures/#dsq-comment-43493993

It should be removed from all plugins and replaced by something like 'that' or '_this'.

Change History

comment:1 follow-up: ↓ 2 Changed 4 years ago by AzaToth

doesn't "var self=this;" define (and declare at the same time) self in the scope? Or is my understanding of javascript totally whacked? (or did you mean simple "self=this;" without the var part)

comment:2 in reply to: ↑ 1 Changed 4 years ago by scott.gonzalez

Replying to AzaToth:

doesn't "var self=this;" define (and declare at the same time) self in the scope? Or is my understanding of javascript totally whacked? (or did you mean simple "self=this;" without the var part)

The problem is if you forget to include the assignment altogether. This isn't that uncommon if code is moved from one function to another. If the original function defines self and the new function doesn't, you could have a problem. In some cases, it's not obvious that the code is broken, because you may not get errors as an undefined self will reference window (try running window.self === window in a console). This could lead to storing values on the window instead of the plugin instance, and your plugin would continue to work properly, but would leak data into the global scope. Using a name other than self reduces the chances of this mistake going unnoticed.

comment:3 Changed 4 years ago by joern.zaefferer

And that's not just a theoretical problem. Cost me a few hours at the time.

comment:4 Changed 4 years ago by rdworth

  • Description modified (diff)

comment:5 Changed 4 years ago by gnarf

  • Status changed from new to open

comment:7 Changed 3 years ago by scott.gonzalez

  • Description modified (diff)

comment:8 Changed 2 years ago by scott.gonzalez

  • Status changed from open to closed
  • Resolution set to fixed
Note: See TracTickets for help on using tickets.