Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incorrect function.name behavior when Object.setPrototypeOf is used #296

Closed
zenorbi opened this issue Apr 25, 2017 · 5 comments
Closed

incorrect function.name behavior when Object.setPrototypeOf is used #296

zenorbi opened this issue Apr 25, 2017 · 5 comments
Labels

Comments

@zenorbi
Copy link

zenorbi commented Apr 25, 2017

TypeScript 2 uses Object.setPrototypeOf to inherit static fields from the base class.

In environments where function.name doesn't exist, this code returns BaseClass, BaseClass. The correct behavior is BaseClass, SubClass.

If the order of the 2 console.log statements are reversed core-js produces the correct results. The problem is that core-js caches the name, so when I call the BaseClass.name a cache gets created and a new property is defined using a static value for the name property instead of the expensive getter. Then when I call SubClass.name, the prototype chain contains the cached value (defined in BaseClass), so the expensive getter won't get called.

var __extends = (this && this.__extends) || (function () {
    var extendStatics = Object.setPrototypeOf ||
        ({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
        function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; };
    return function (d, b) {
        extendStatics(d, b);
        function __() { this.constructor = d; }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    };
})();
var BaseClass = (function () {
    function BaseClass() {
    }
    return BaseClass;
}());
var SubClass = (function (_super) {
    __extends(SubClass, _super);
    function SubClass() {
        return _super !== null && _super.apply(this, arguments) || this;
    }
    return SubClass;
}(BaseClass));
console.log(BaseClass.name);
console.log(SubClass.name);

Possible fixes:

  • remove the cache (obviously would be a bit too expensive)
  • define a new getter instead of a static value cache which checks if the this is that this of the cached value. If the check fails, the new name would be calculated and if would define a new property for this new this value.
@zenorbi
Copy link
Author

zenorbi commented Apr 25, 2017

The same issue happens then using babel's es2015 preset.

"use strict";

function _possibleConstructorReturn(self, call) { if (!self) { throw new ReferenceError("this hasn't been initialised - super() hasn't been called"); } return call && (typeof call === "object" || typeof call === "function") ? call : self; }

function _inherits(subClass, superClass) { if (typeof superClass !== "function" && superClass !== null) { throw new TypeError("Super expression must either be null or a function, not " + typeof superClass); } subClass.prototype = Object.create(superClass && superClass.prototype, { constructor: { value: subClass, enumerable: false, writable: true, configurable: true } }); if (superClass) Object.setPrototypeOf ? Object.setPrototypeOf(subClass, superClass) : subClass.__proto__ = superClass; }

function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }

var BaseClass = function BaseClass() {
  _classCallCheck(this, BaseClass);
};

var SubClass = function (_BaseClass) {
  _inherits(SubClass, _BaseClass);

  function SubClass() {
    _classCallCheck(this, SubClass);

    return _possibleConstructorReturn(this, (SubClass.__proto__ || Object.getPrototypeOf(SubClass)).apply(this, arguments));
  }

  return SubClass;
}(BaseClass);

console.log(BaseClass.name);
console.log(SubClass.name);

@zloirock
Copy link
Owner

You are right, but I'm not sure which behaviour is more correct - without adding own .name property to function and which works with prototype chains, like in your case, or with adding, like it's required by the spec. Most likely, I'll remove caching in the next major release.

@zloirock zloirock added the es6 label Apr 29, 2017
@Drabc
Copy link

Drabc commented Aug 2, 2017

I'm experiencing a similar issue and I would like to know if it is related to this problem. What is interesting is that this behavior only occurs on IE11(only IE I tested). I'm using Typescript 2 in an Angular project. I have an abstract class A which extends from Angular's ErrorHandler. Then I have another class B which extends from A.

abstract class A extends ErrorHandler {
  ...
}

class B extends A {
  ...
}

Now when I try B.name I get ErrorHandler. Now, If I remove the ErrorHandler Inheritance:

abstract class A {
  ...
}

then when I try B.name I get B

Here are all the corejs files I'm including.

import 'core-js/es6/symbol'
import 'core-js/es6/object'
import 'core-js/es6/function'
import 'core-js/es6/parse-int'
import 'core-js/es6/parse-float'
import 'core-js/es6/number'
import 'core-js/es6/math'
import 'core-js/es6/string'
import 'core-js/es6/date'
import 'core-js/es6/array'
import 'core-js/es6/regexp'
import 'core-js/es6/map'
import 'core-js/es6/set'
import 'core-js/es6/weak-map'
import 'core-js/es6/weak-set'
import 'core-js/es6/typed'
import 'core-js/es6/reflect'

import 'core-js/es7/reflect'
import 'core-js/es7/array'

import 'core-js/fn/object/values'

@zenorbi
Copy link
Author

zenorbi commented Aug 2, 2017

@Drabc it's related. The issue isn't on how deep is your prototype chain or what is in it but rather the order in which you query the name properties. Once you query the name property, core-js caches its value and every other "inherited" class hits the cached name before hitting the actual shim that would calculate it. Until the patch lands comment out the following line in your core-js copy:

has(that, NAME) || !isExtensible(that) || dP(that, NAME, createDesc(5, name));

Just make sure you don't query the name property too often as this comment removes the cache.

@zloirock
Copy link
Owner

zloirock commented Aug 4, 2017

OK, I'll remove caching without a major bump, seems it should not cause any problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants