Playing with Google's Closure Compiler and got a message for Prototype 1.7.0.0

[Richard Quadling]

Hi.

Using Google Maps API with Prototype. Playing putting it into Google
Closure Compiler.

Got a message ...

Number of warnings: 1

JSC_USELESS_CODE: Suspicious code. This code lacks side-effects. Is
there a bug? at line 4437 character 3 in prototype.js
elem.parentNode.selectedIndex;
^

which is ...

filters: {
enabled: function(elem){
return elem.disabled === false && elem.type !== "hidden";
},
disabled: function(elem){
return elem.disabled === true;
},
checked: function(elem){
return elem.checked === true;
},
selected: function(elem){
elem.parentNode.selectedIndex; // <<<<<<<< line 4437
return elem.selected === true;
},
parent: function(elem){
return !!elem.firstChild;
},
empty: function(elem){
return !elem.firstChild;
},

It looks like they are right? From what I know, that statement isn't
going to do anything.

To see this in action...

Go to http://closure-compiler.appspot.com/home and enter ...

// ==ClosureCompiler==
// @compilation_level SIMPLE_OPTIMIZATIONS
// @output_file_name default.js
// @code_url http://ajax.googleapis.com/ajax/libs/prototype/1.7.0.0/prototype.js
// @code_url http://maps.google.com/maps/api/js?libraries=geometry&sensor=false
// ==/ClosureCompiler==

// ADD YOUR CODE HERE
alert(1);

as the code to optimize.

Choose "Simple" from the list of optimizations.

Click "Compile" and then look at the warnings tab on the right hand side.

Regards,

Richard.

--
Richard Quadling
Twitter : EE : Zend
@RQuadling : e-e.com/M_248814.html : bit.ly/9O8vFY

[Andrew Dupont]

I had to look it up, because that's part of Sizzle [1], but accessing that property does have a side effect, apparently. It makes the `selected` property work correctly when something is selected by default.

That strikes me as a spurious thing for GCC to warn about — you wouldn't typically expect property access to have side effects, but with getters/setters now in JavaScript, it'll become more and more common.

Cheers,
Andrew

[1] https://github.com/jquery/sizzle/blob/master/sizzle.js#L598

> --
> You received this message because you are subscribed to the Google Groups "Prototype: Core" group.
> To post to this group, send email to prototy...@googlegroups.com
> To unsubscribe from this group, send email to prototype-cor...@googlegroups.com
> For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en

[Rick Waldron]

Andrew, Richard

Not sure if this helps, but I figured it was worth the mention... jQuery no longer uses GCC for min/compression... but when we did, we had the warn level set to quiet:

https://github.com/jquery/jquery/commit/d503845d0cf45632c0d7c3542ffd1b19257a8e5e#L0L104

Hopefully that's useful information :)

Rick

[Nicolás Sanguinetti]

I like that it warns about *lack* of side effects. I mean, any
functional programmer would think that's a great warning to have :)

-foca

[Richard Quadling]

Using a getter to alter state sounds like a really bad idea.

Thank you all for looking at this. Glad it wasn't anything important.

OOI, the Sizzle code wraps the access in a condition. Should this be
ported to Prototype?

[Andrew Dupont]

On May 18, 2011, at 3:22 AM, Richard Quadling wrote:

> OOI, the Sizzle code wraps the access in a condition. Should this be
> ported to Prototype?

We'll update to the latest version of Sizzle when we release 1.7.0.1.

[Richard Quadling]

Thank you all for your time.