google / closure-compiler

A JavaScript checker and optimizer.
https://developers.google.com/closure/compiler/
Apache License 2.0
7.4k stars 1.15k forks source link

Element.style.float improperly compiled #2405

Open Datamance opened 7 years ago

Datamance commented 7 years ago

Hi there,

Here's a little TODO from my codebase:

// TODO(rrodriguez): report compiler bug - obfuscation of "float" property.
  leftHelper.style['float'] = 'left';
  leftHelper.innerText = 'Not at all likely';

When I compile, the element doesn't float properly. Unminified, the element does exactly what it's supposed to. This happens anywhere I try Element.style.float. It does not occur for any other style properties. So, this probably means that the "float" style property is being treated like a regular old property for some reason.

Here's my compiler info:

VERSION: 20161201.0.0

FLAGS: java -jar node_modules/google-closure-compiler/compiler.jar --js="nps-plugin/index.js" --js="node_modules/google-closure-library/closure/goog/.js" --js="!node_modules/google-closure-library/closure/goog/_test.js" --dependency_mode=STRICT --compilation_level=ADVANCED_OPTIMIZATIONS --entry_point=lnkd.NpsPlugin --output_wrapper="(function(){%output%})();" --new_type_inf --jscomp_warning=newCheckTypes --jscomp_off=newCheckTypesExtraChecks --hide_warnings_for=node_modules/google-closure-library/ --output_manifest="build/report/manifest.MF" --js_output_file=nps-plugin.js

Let me know if you can repro. Thanks!

MatrixFrog commented 7 years ago

The bracket notation tells the compiler not to rename it. Use leftHelper.style.float (which is invalid in ES3 but works in modern browsers). If you need to run in ES3 browsers you can set the output language to ES3 and it will add the brackets back in the output.

ChadKillingsworth commented 7 years ago

I believe you are losing type information somewhere. float is definitely defined in the externs and isn't renamed.

Datamance commented 7 years ago

Not sure why this was closed.

@MatrixFrog you misunderstand my problem :) it is granted that the compiler will rename unbracketed properties, but certain DOM APIs should have accompanying built-in externs - Element.style.float being one of them.

@ChadKillingsworth How would I be losing type information? If I were losing type information, wouldn't the other style settings break?

I have lots of code like this:

  textAreaLabelStyle.display = userNameLabelStyle.display = 'block';
  textAreaLabelStyle.fontSize = userNameLabelStyle.fontSize = '12px';
  textAreaLabelStyle.color = userNameLabelStyle.color = '#696c6f';

wouldn't that break as well?

ChadKillingsworth commented 7 years ago

A good way to check is to set the flag --use_types_for_optimization=false. If that fixes the issue, then you are somehow lying to the compiler about types (this is far easier to do than it might seem).

The float property of styles is defined in the externs and most definitely is not renamed in general. Something else is going on.

Datamance commented 7 years ago

@ChadKillingsworth tried it, same issue. The float property gets obfuscated and I lose the styling.

Here's the docstring and relevant code:

/**
 * Creates the submit button.
 * @return {Element}
 * @private
 */
lnkd.NpsPlugin.createSubmitButton_ = function() {
  var submitButton = document.createElement('input'),
      submitStyle = submitButton.style;

... and then at the bottom of the function...

  submitStyle.letterSpacing = '0.7px';
  // TODO(rrodriguez): report this bug!
  submitStyle['float'] = 'right';

  return submitButton;
};

Is Element not the right type here?

ChadKillingsworth commented 7 years ago

I managed to reproduce this. The problem is historically the property is called styleFloat or cssFloat as float is a reserved keyword and cannot be used as a property name in ES3.

It looks like it just needs added to our externs now. A PR would be welcome.