Parent class constructor evaluated when defining child class

[visionscaper]

Hello,

Thank you for writing jsface, I think it is a powerful OOP library for JS.

I came across the following issue that occurs in 2.3. but not in 2.2.

In the following simplified code a child class C is defined based on a parent class P. Defining C results in the constructor of P to be called, see the 'No Name!!' output.

Calling the constructor of parent P when defining child class C is unwanted behaviour in my opinion. A fix would be awesome!

var Class = require('jsface').Class
var P = Class({
    constructor : function(name) {
        if (!name) {
            console.error('No Name!!');
        } else {
            console.info('OK');
        }
    }
});

//We define child class C here based on parent class P, which results in 
// the constructor of P to be called
var C = Class(P, {
    constructor : function() {
        C.$super.call(this, "TEST");
    }
});

//CONSOLE OUTPUT:
No Name!!

var c = new C()

//CONSOLE OUTPUT:
OK

[visionscaper]

Hi @tnhu,

Were you able to check out this bug? It seems to be introduced with fixing #20. You added the following line: clazz.prototype = classOrNil(parent[0]) ? new parent[0] : parent[0];

When classOrNil returns true this evaluates the constructor of the parent class while the class is actually not constructed at that point in time but merely defined.

Please let me know what you think!

[visionscaper]

Hi again @tnhu,

I solved this issue myself. By replacing in jsface.js :

clazz.prototype = classOrNil(parent[0]) ? new parent[0] : parent[0];

by

clazz.prototype.__proto__  = classOrNil(parent[0]) ? parent[0].prototype : parent[0];

Can you replace it in your code? Or should I send a pull request?

Cheers,

-- Freddy Snijder

[tnhu]

Hi Freddy,

Thanks for reporting. I apologize for being super late. Been terribly busy with work so do not have time to look into jsface bugs. I probably have some time mid-next week. I'll try to find a fix for this by then.

Your solution could not work across platform since __proto__ is not a standardized property.

Again, very sorry for the late!

Tan

[visionscaper]

Hi Tan,

Thanks for reacting. You are right, proto is not (yet) a standardised property.

I did a quick test and this works as well (not using proto):

var __inherit = function(childClass, parentClass) {
    var f=function(){}; // defining temp empty function
    f.prototype=parentClass.prototype;
    f.prototype.constructor=f;

    childClass.prototype=new f;

    childClass.prototype.constructor=childClass; // restoring proper constructor for child class
    parentClass.prototype.constructor=parentClass; // restoring proper constructor for parent class
};

if (classOrNil(parent[0])) {
    __inherit(clazz, parent[0]);
} else {
    clazz.prototype = parent[0];
}

I'm looking forward to see the final fix!

-- Freddy Snijder

[tnhu]

I have a better patch, can you apply it and see if it works for you?

    if ( !singleton && len) {
      var parentClass = classOrNil(parent[0]);

      if ( !parentClass) {
        clazz.prototype = parent[0];
      } else {
        for (var key in parentClass) {
          if ( !ignoredKeys[key]) {
            clazz.prototype[key] = parentClass[key];
          }
        }
      }
      clazz.prototype.constructor = clazz;
    }

[visionscaper]

Hi @tnhu,

I'm sorry, no not completely. Although this issue is solved with that code, it does break issue #20 again. I tested my last solution (and my first solution) by trying to recreate this issue and issue #20 and running the sample code in your readme.md. My solutions did work, for this issue and for issue #20.

Good luck!

Cheers,

FS

[tnhu]

Hi Freddy, Thanks for the fix. I merged your solution into 2.4.0. Credit on CHANGELOG :) Let me know you found anything else.

Tan

[visionscaper]

That's great @tnhu, thanks!