Extending doop classes multiple times results in $__Doops__$Indices pollution

[doopuser]

It seems that when extending a @doop class multiple times the _super object's $Doops$Indices gets polluted with the superset of all extending class @doop properties. I believe this is because we are copying the super's reference when extending the $Doops$Indices object in doop().

var indices = target.$__Doops__$Indices || (target.$__Doops__$Indices = {});
indices[propertyKey] = index;

If we look in the console, we can verify that the references are indeed the same: target.$Doops$Indices === ParentClass.prototype.$Doops$Indices

Thus when we set the propertyKey, we end up adding it to both the sub-class and parent-class. Each additional subclass pollutes the array further as they are all sharing the same reference. However, because target.$Doops$Count is a literal, it is not copied by reference, and thus is unique per subclass. At runtime this causes very strange bugs - whoever set the index value last, wins.

A quick-n-dirty solution I wrote is to crudely copy the object at runtime and re-assign it to break the reference.

  var indices = {};
  if (target.$__Doops__$Indices)
  {
      indices = JSON.parse(JSON.stringify(target.$__Doops__$Indices))
  }

  indices[propertyKey] = index;
  target.$__Doops__$Indices = indices;

I have not submitted a pull request because this is a terribly inefficient way of copying objects, I hope that you know a far more clever way than my rudimentary JS skills can provide.

To repro the issue, I used the following sample code:

@doop
class Animal {
    @doop
    get hasLegs() {
        return doop<boolean, this>();
    }

    @doop
    get hasArms() {
        return doop<boolean, this>();
    }
}

@doop
class Beaver extends Animal {
    @doop
    get hasTail() {
        return doop<boolean, this>();
    }
}

@doop
class Squirrel extends Animal {
    @doop
    get hasTeeth() {
        return doop<string, this>();
    }
}
let beaver = new Beaver();
let squirrel = new Squirrel();

When you look inside either the Beaver or Squirrel object you'll see:

{
   [functions]: ,
   $__Doops__$Constructing: 0,
   $__Doops__$Count: 3,
   $__Doops__$Indices: {
      [functions]: ,
      __proto__: { },
      hasArms: 1,
      hasLegs: 0,
      hasTail: 2,
      hasTeeth: 2
   },
   __proto__: { }
}

They should each only have 3 properties, not 4. Notice that the indices collide.

[danielearwicker]

I found a way to show actual confusion of the values of the fields in objects due to this bug. I've updated the tests accordingly. In your example it doesn't actually cause a problem because hasTail and hasTeeth never appear in the same object. If you're interested, in DoopSpec.ts there is now a class SuperPiglet that exhibits actual corruption due to two different fields in the same record sharing the same storage slot. It has to derive from Piglet (that's when the corrupted indices map is actually used for the first time) and Piglet and Pooh have to be declared in a certain order, and also their properties! It's a very sensitive combination of things.

By the way, your method of cloning via JSON is quite well-regarded - browsers implement it natively so it's one of the fastest general cloning techniques. And this clone only needs to happen when setting up a class (not on each property, and certainly not per-instance which would be terrible for performance - the whole point of doop is to only depend on Array#slice which is super-fast).

Thanks for your help with this. Please upgrade to 0.9.3.

[doopuser]

Fantastic, thanks!