ksylvest/jquery-growl

option name `static` is js reserved keyword

jhargis opened this issue · 6 comments

I believe it may be bad form to use js reserved words as object properties such as static.
I've noticed that the YUI compressor fails when it encounters the property in this form

if (data.static === true)

but can be overcome with dict notation

data['static']

output from the compressor looks like this

Failed, error was: ExternalTool: subprocess returned a non-success result code: 2, stdout=, stderr=[ERROR] in -
  1884:28:missing name after . operator

Since using a reserved word is technically permitted in ECMAScript 5 it IS legal, but it can be problematic in certain cases such as this.

see http://stackoverflow.com/questions/7022397/using-reserved-words-as-property-names-revisited#answer-18533285

I suspect changing it would be rather disruptive. Perhaps a warning ?

thoughts?

Good catch. Didn't realize is was a reserved word. What do you think about using fixed or anchored as a replacement? Seems like a better option anyways.

I like them both! fixed slightly better just because it's shorter. I'd suggest doing something like leaving static in and throwing a deprecation warning in console. Documenting it as a deprecated option and leaving it for a few releases before yanking it entirely. That way everybody using the library will have time to update their code and hopefully recognize that it's going away. Until such time as it is removed, both options have the same effect but the primary documentation focuses on the new name fixed or anchored.

cheers!

@jhargis Pull request here: #27

If you can confirm this looks correct I can do a new release (1.2.6).

reviewed

So responsive! thank you! great plugin Kevin!

Thanks for finding.