Leaking empty objects when instanciating (sub)subclasses

pogman-code commented 9 years ago

Hi, first of all, here is a simple test case:

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8"/>
</head>
<body>
<script src="stapes.js"></script>
<script>
  var A = Stapes.subclass({
    constructor: function () {
      A.parent.constructor.call(this);
    }
  });
  var B = A.subclass({
    constructor: function () {
      B.parent.constructor.call(this);
    }
  });
  var C = B.subclass({
    constructor: function () {
      C.parent.constructor.call(this);
    }
  });

  var o1 = new A(); // _guid === 1
  var o2 = new B(); // _guid === 3
  var o3 = new C(); // _guid === 6
</script>
</body>
</html>

Due to the mechanism of Stapes, when instantiating a subclass that calls its parent's constructor, it adds unnecessary empty objects to .attributes and .eventHandlers. As far as we understood this issue is due to calls to _.addGuid( this, true ) that are triggered through the dynamically generated constructor of subclasses.

In the example above, we can see that the deeper our subclasses are, the more we leak (and that's understandable).

At first I thought of using

// If this class has events add a GUID as well
if (this.on) {
    _.addGuid( this, true );
}

As default constructor of _.Module (the default Stapes superclass) and use the following as realConstructor:

var realConstructor = props.hasOwnProperty('constructor') ? props.constructor : superclass.constructor;

The thing with this solution is that it break "events on subclasses" unit test because Parent class' constructor does not call its superclass (Stapes) constructor. Adding

Parent.parent.constructor.call(this);

Fixes this test.

To end this ticket, it would be great if you can check that yourselves because we might not understand all inherent mechanism of Stapes and there might be a better solution to this.

Thanks. (And great piece of work anyway!)

pogman-code commented 9 years ago

An example is pretty simple. On the previous example, just set a class variable within A's constructor. This variable won't be available on a new B() object if we don't call B. parent.constructor.call(this); within B's constructor.

i.e.:

var A = Stapes.subclass({ constructor: function () { this.a = "I'm a"; A.parent.constructor.call(this); } }); var B = A.subclass({ constructor: function () { this.b = "I'm b"; } }); var o2 = new B(); // _guid === 3

In the end o2.a is undefined

2015-02-14 15:38 GMT+01:00 Anders notifications@github.com:

Could you provide a case where you need to call the parent's constructor like this? As far as I can see, what you're trying to do, is already handled internally by Stapes.

— Reply to this email directly or view it on GitHub https://github.com/hay/stapes/issues/60#issuecomment-74377448.

supermensa commented 9 years ago

Yup, I'm with you and agree. Should be added to the docs as well.

foxx commented 9 years ago

@Jesus-21 Could you put together a PR and unit test for this?

hay / stapes

Leaking empty objects when instanciating (sub)subclasses #60