Haxe 4 js.Bool.__interfLoop breaking change for Interface casting when using advanced JavaScript minification

[derRaab]

I came across an issue while using the closure compiler --compilation_level ADVANCED_OPTIMIZATIONS but I would expect other minification utilities like uglifyjs to break as well.

Casting to classes works fine, but casting to interfaces doesn't work anymore. I tracked the breaking change down to this commit: 3e46bfa#diff-b82d472d6b1e1b69e1560649a3bf52d7

I recently switched to from Haxe 3.4.7 to 4.0.5 and found our minified production JavaScript throws an Cannot cast IMPLEMENTATION to INTERFACEerror.

Here is a Haxe code example:

package ;
import js.Browser;

interface Api {
 public function getString() : String;
}

class ApiClass implements Api {
 inline public function new() {}
 public function getString() : String {
  return "String";
 }
}

class ApiClassSub extends ApiClass {
 inline public function new() { super(); }
}

class ApplicationJsMain {
 static function main() {
  var apiClass : ApiClass = new ApiClass();
  var apiClassSub : ApiClassSub = new ApiClassSub();
  Browser.console.log( "apiClassSub.method():             " + apiClassSub.getString() );
  Browser.console.log( "apiClass.method():                " + apiClass.getString() );

  var apiClassFromApiClassSub : ApiClass = cast( apiClassSub, ApiClass ); // WORKS
  Browser.console.log( "apiClassFromApiClassSub.method(): " + apiClassSub.getString() );
  var apiFromApiClass : Api = cast( apiClass, Api ); // CRASHES
  Browser.console.log( "apiFromApiClass.method():         " + apiClassSub.getString() );
 }
}

Which compiles to nicely to JavaScript and works just fine ( I only paste the necessary part ):

js_Boot.__interfLoop = function(cc,cl) {
 if(cc == null) {
  return false;
 }
 if(cc == cl) {
  return true;
 }
 if(Object.prototype.hasOwnProperty.call(cc,"__interfaces__")) {
  var intf = cc.__interfaces__;
  var _g = 0;
  var _g1 = intf.length;
  while(_g < _g1) {
   var i = _g++;
   var i1 = intf[i];
   if(i1 == cl || js_Boot.__interfLoop(i1,cl)) {
    return true;
    }
  }
 }
 return js_Boot.__interfLoop(cc.__super__,cl);
};

But if we let closure compiler minify the JavaScript, there is never a property called " __ interfaces __ ":

function c() {}
c.c = function (a, b) {
 if (null == a)return !1;
 if (a == b)return !0;
 if (Object.prototype.hasOwnProperty.call(a, "__interfaces__"))for (var d = a.m, f = 0, e = d.length; f < e;) {
  var k = f++;
  k = d[k];
  if (k == b || c.c(k, b))return !0
 }
 return c.c(a.g, b)
};

var intf = cc.__interfaces__; becomes var d = a.m.

I'm not a 100% sure what to do next - I just wanted to open an issue.

My very first fix is to avoid save casting to Interfaces, but since it worked with 3.4.7 I think it might be a good idea so ensure advanced minification support?

[derRaab]

Is it possible to change this:

// JavaScript riddle:
    // Not minified:
    var obj = {};
        obj.key = "value";
    if ( obj.hasOwnProperty( "key" ) ) {

        var value = obj.key;
    }
    // Minified:
    var o = {};
        o.k = "value";
    if ( o.hasOwnProperty( "key" ) ) { // <-- how to change that "key" to "k" based on the minified code?

        var v = o.k;
    }

[RealyUniqueName]

Does closure compiler have any options to not minify specific field names?

[derRaab]

Absolutely. Uglify JS as well. But with a very big code base that's not an option. :)

[RealyUniqueName]

I'm surprised they don't handle hasOwnProperty with constant arguments automatically.

[derRaab]

Closure compiler? We would need something like getPropertyNameFromJavaScriptCode that returns just "k" in this case.

if ( o.hasOwnProperty( getPropertyNameFromJavaScriptCode( o.k ) ) ) {
}

[nadako]

what if we do cc["__interfaces__"] instead?

[derRaab]

That wouldn't help since it would be undefined.

[derRaab]

We would need to do that for every access point everywhere.

Actually that might apply to the "__ class __" and other keys as well?

[nadako]

I guess that depends on whether we do the check by the string like in the hasOwnProperty case or not.

[back2dos]

How about we change the check to !!cc.__interfaces__?

I wonder why the code is using hasOwnProperty in the first place. Class object are just functions, so their __proto__ is Function.prototype, meaning there's practically no way __interfaces__ could be inherited. Then again, I could be missing something.

[Simn]

See #7834 for the original issue.

[back2dos]

Ok, seems to be an ES6 thing. Yay.

How about we leverage the type information we have? It would seem to produce the correct result:

interface I1 {}
interface I2 {}
interface O1 {}

class Base1 implements I1 implements I2 {
  public function new() {}
}

class Base2 extends Base1 {}
class Base3 extends Base2 {}

class Main {
  static public function main() {
    untyped {
      js.Boot.__interfLoop = function __interfLoop(cc:Dynamic, cl:Dynamic) {
        if (cc != null && cl != null)
          trace(Type.getClassName(cc), Type.getClassName(cl));
        if (cc == null)
          return false;
        if (cc == cl)
          return true;
        var intf:Dynamic = cc.__interfaces__;
        if (intf #if (js_es == 6) && (cc.__super__ == null || cc.__super__.__interfaces__ != intf) #end) {
          for (i in 0...intf.length) {
            var i:Dynamic = intf[i];
            if (i == cl || __interfLoop(i, cl))
              return true;
          }
        }
        return __interfLoop(cc.__super__, cl);
      }
    }

    var base3 = new Base3();
    trace(Std.is(base3, O1));

    var cc:Dynamic = Base3;
    haxe.Timer.measure(function () {
      var x = false;
      for (i in 0...10000000)
        x = js.lib.Object.prototype.hasOwnProperty.call(cc, "__interfaces__");
      if (Math.random() > 42) trace(x);
    });// Main.hx:38: 0.12400007247924805s on node v10.15.3

    haxe.Timer.measure(function () {
      var x = false;
      for (i in 0...10000000) {
        var intf:Dynamic = cc.__interfaces__;
        x = intf && (cc.__super__ == null || cc.__super__.__interfaces__ != intf);
      }
      if (Math.random() > 42) trace(x);
    });// Main.hx:45: 0.015000104904174805s on node v10.15.3


  }
}

FWIW it also seems to be faster than native reflection.

[nadako]

I'll have to look into this again, I don't see right now why it's an ES6 thing, but I like the idea of not using hasOwnProperty :)

[back2dos]

It's an ES6 thing, because without it, there's no prototype chain between class objects, as we have nothing in the output that would establish it. I'd say it's a design flaw in ES6 classes, but oh well. In any case, you can best observe the difference here:

class A { static public var foo = 123; }
class B extends A {}

class Main {
  static public function main() {
    trace(js.lib.Object.getPrototypeOf(B) == A);// true with -D js_es=6, false otherwise
    trace(untyped B.foo);// 123 with -D js_es=6, undefined otherwise
  }
}

Note that without the untyped it won't compile, because Haxe treats class objects in a saner manner.