Wrongly added `Array.prototype.keys` polyfill when `Object.keys` is used

[lahmatiy]

When Object.keys() is used, closure compiler adds Array.prototype.keys polyfill. Test file:

function test(o) {
    return Object.keys(o);
}

Output:

var $jscomp=$jscomp||{};$jscomp.scope={};$jscomp.defineProperty="function"==typeof Object.defineProperties?Object.defineProperty:function(a,c,b){if(b.get||b.set)throw new TypeError("ES3 does not support getters and setters.");a!=Array.prototype&&a!=Object.prototype&&(a[c]=b.value)};$jscomp.getGlobal=function(a){return"undefined"!=typeof window&&window===a?a:"undefined"!=typeof global&&null!=global?global:a};$jscomp.global=$jscomp.getGlobal(this);$jscomp.SYMBOL_PREFIX="jscomp_symbol_";
$jscomp.initSymbol=function(){$jscomp.initSymbol=function(){};$jscomp.global.Symbol||($jscomp.global.Symbol=$jscomp.Symbol)};$jscomp.symbolCounter_=0;$jscomp.Symbol=function(a){return $jscomp.SYMBOL_PREFIX+(a||"")+$jscomp.symbolCounter_++};
$jscomp.initSymbolIterator=function(){$jscomp.initSymbol();var a=$jscomp.global.Symbol.iterator;a||(a=$jscomp.global.Symbol.iterator=$jscomp.global.Symbol("iterator"));"function"!=typeof Array.prototype[a]&&$jscomp.defineProperty(Array.prototype,a,{configurable:!0,writable:!0,value:function(){return $jscomp.arrayIterator(this)}});$jscomp.initSymbolIterator=function(){}};$jscomp.arrayIterator=function(a){var c=0;return $jscomp.iteratorPrototype(function(){return c<a.length?{done:!1,value:a[c++]}:{done:!0}})};
$jscomp.iteratorPrototype=function(a){$jscomp.initSymbolIterator();a={next:a};a[$jscomp.global.Symbol.iterator]=function(){return this};return a};$jscomp.iteratorFromArray=function(a,c){$jscomp.initSymbolIterator();a instanceof String&&(a+="");var b=0,d={next:function(){if(b<a.length){var e=b++;return{value:c(e,a[e]),done:!1}}d.next=function(){return{done:!0,value:void 0}};return d.next()}};d[Symbol.iterator]=function(){return d};return d};
$jscomp.polyfill=function(a,c,b,d){if(c){b=$jscomp.global;a=a.split(".");for(d=0;d<a.length-1;d++){var e=a[d];e in b||(b[e]={});b=b[e]}a=a[a.length-1];d=b[a];c=c(d);c!=d&&null!=c&&$jscomp.defineProperty(b,a,{configurable:!0,writable:!0,value:c})}};$jscomp.polyfill("Array.prototype.keys",function(a){return a?a:function(){return $jscomp.iteratorFromArray(this,function(a){return a})}},"es6-impl","es3");function test(a){return Object.keys(a)};

I believe it's incorrect.

> java -jar  compiler.jar --version
Closure Compiler (http://github.com/google/closure-compiler)
Version: v20170409
Built on: 2017-04-11 10:26

[MatrixFrog]

In general we will sometimes add polyfills we don't need, though we can probably do better in this case. cc @shicks

[gaearon]

This didn't use to be an issue for us, but after updating to 20171203, my bundle now contains this (prettified):

"use strict";
var $jscomp = $jscomp || {};
$jscomp.scope = {};
$jscomp.owns = function(obj, prop) {
  return Object.prototype.hasOwnProperty.call(obj, prop);
};
$jscomp.ASSUME_ES5 = !1;
$jscomp.ASSUME_NO_NATIVE_MAP = !1;
$jscomp.ASSUME_NO_NATIVE_SET = !1;
$jscomp.defineProperty =
  $jscomp.ASSUME_ES5 || "function" == typeof Object.defineProperties
    ? Object.defineProperty
    : function(target, property, descriptor) {
        target != Array.prototype &&
          target != Object.prototype &&
          (target[property] = descriptor.value);
      };
$jscomp.getGlobal = function(maybeGlobal) {
  return "undefined" != typeof window && window === maybeGlobal
    ? maybeGlobal
    : "undefined" != typeof global && null != global ? global : maybeGlobal;
};
$jscomp.global = $jscomp.getGlobal(this);
$jscomp.polyfill = function(target, polyfill) {
  if (polyfill) {
    var obj = $jscomp.global;
    target = target.split(".");
    for (var i = 0; i < target.length - 1; i++) {
      var key = target[i];
      key in obj || (obj[key] = {});
      obj = obj[key];
    }
    target = target[target.length - 1];
    i = obj[target];
    polyfill = polyfill(i);
    polyfill != i &&
      null != polyfill &&
      $jscomp.defineProperty(obj, target, {
        configurable: !0,
        writable: !0,
        value: polyfill
      });
  }
};
$jscomp.polyfill(
  "Object.assign",
  function(orig) {
    return orig
      ? orig
      : function(target, var_args) {
          for (var i = 1; i < arguments.length; i++) {
            var source = arguments[i];
            if (source)
              for (var key in source)
                $jscomp.owns(source, key) && (target[key] = source[key]);
          }
          return target;
        };
  },
  "es6",
  "es3"
);
$jscomp.SYMBOL_PREFIX = "jscomp_symbol_";
$jscomp.initSymbol = function() {
  $jscomp.initSymbol = function() {};
  $jscomp.global.Symbol || ($jscomp.global.Symbol = $jscomp.Symbol);
};
$jscomp.Symbol = (function() {
  var counter = 0;
  return function(opt_description) {
    return $jscomp.SYMBOL_PREFIX + (opt_description || "") + counter++;
  };
})();
$jscomp.initSymbolIterator = function() {
  $jscomp.initSymbol();
  var symbolIterator = $jscomp.global.Symbol.iterator;
  symbolIterator ||
    (symbolIterator = $jscomp.global.Symbol.iterator = $jscomp.global.Symbol(
      "iterator"
    ));
  "function" != typeof Array.prototype[symbolIterator] &&
    $jscomp.defineProperty(Array.prototype, symbolIterator, {
      configurable: !0,
      writable: !0,
      value: function() {
        return $jscomp.arrayIterator(this);
      }
    });
  $jscomp.initSymbolIterator = function() {};
};
$jscomp.arrayIterator = function(array) {
  var index = 0;
  return $jscomp.iteratorPrototype(function() {
    return index < array.length
      ? { done: !1, value: array[index++] }
      : { done: !0 };
  });
};
$jscomp.iteratorPrototype = function(next) {
  $jscomp.initSymbolIterator();
  next = { next: next };
  next[$jscomp.global.Symbol.iterator] = function() {
    return this;
  };
  return next;
};
$jscomp.iteratorFromArray = function(array, transform) {
  $jscomp.initSymbolIterator();
  array instanceof String && (array += "");
  var i = 0,
    iter = {
      next: function() {
        if (i < array.length) {
          var index = i++;
          return { value: transform(index, array[index]), done: !1 };
        }
        iter.next = function() {
          return { done: !0, value: void 0 };
        };
        return iter.next();
      }
    };
  iter[Symbol.iterator] = function() {
    return iter;
  };
  return iter;
};
$jscomp.polyfill(
  "Array.prototype.keys",
  function(orig) {
    return orig
      ? orig
      : function() {
          return $jscomp.iteratorFromArray(this, function(i) {
            return i;
          });
        };
  },
  "es6",
  "es3"
);

I didn't change any options.

You can reproduce by installing Yarn and then running

git clone https://github.com/gaearon/react.git
cd react
git checkout bump-gcc
yarn
yarn build core --type=FB_DEV

The build output is in build/facebook-www/React-prod.js (it's prettified for readability).

This is a blocker for updating for us: we can't afford to ship any polyfills.

[lahmatiy]

@gaearon After I filled the issue, I found the solution a few months ago. If --no-rewritePolyfills option is passed, GCC stops adding a polyfill prelude. Not sure it still working, but maybe it helps to you too.

[gaearon]

I’m using the JS version that claims rewritePolyfills is set to false by default. I assume it’s the exact opposite? Or maybe it’s a separate flag that isn’t exposed in JS version? Regardless, thanks for the pointer!

[gaearon]

This is still an issue with 20180402.0.0 and is blocking us from upgrading beyond last year’s September version. Is there anything I could do to help move this forward?

[gaearon]

Hmm. I explicitly set rewritePolyfills to false and indeed it helped.
The documentation was wrong (thanks @nazar-pc for fixing it!)

[shicks]

I was unable to reproduce this with the current compiler. My guess is that the type information on Object was insufficiently precise for the compiler to remove the polyfill (it adds it unconditional on type information, then fails to remove it if it can't prove it's not an Array), but that either the restructuring we did with polyfill removal a year or so ago, or else improved type information on namespaces, has since caused the issue to go away.

If this is still a problem, please reopen.

[westc]

I am still getting this issue while using closure-compiler-v20210406.jar but I found a workaround by simply changing the code to use bracket notation:

function test(o) {
    return Object["keys"](o);
}