-
-
Notifications
You must be signed in to change notification settings - Fork 348
Conversation
ah, includes previous fix for recursive calls, that one was independent from inheritance |
and includes unit test for inheritance, for cases where:
|
Actually even improved it so recursive calls are actually made rather than stopped altogether, but still ensures that mixins are only executed once after all the recursive calls are finished. I'm confident this new implementation should be much more resiliant |
@mweststrate pretty much done, now even supporting weird use cases like a base class that uses a prototype method and the extended class uses an arrow function that calls super on the prototype method |
Btw, the define a property when a property is set in another "this" is basically how JS does things normally internally. From https://developer.mozilla.org/en-US/docs/Web/JavaScript/Inheritance_and_the_prototype_chain Setting a property to an object creates an own property. The only exception to the getting and setting behavior rules is when there is an inherited property with a getter or a setter. Example: function FC() { this.a = 10;};
FC.prototype.a = 5;
FC.prototype.b = 10;
const fc = new FC();
Object.getOwnPropertyDescriptor(fc, "a") // some descriptor
Object.getOwnPropertyDescriptor(fc, "b") // undefined
fc.b = 20
Object.getOwnPropertyDescriptor(fc, "b") // some descriptor |
@xaviergonz looks solid, good tests, but also complicated. Do you feel comfortable about it or do you fear more potential edge cases? If so, I'm still not against classic monkey patching, despite breaking arrow function lifecycles. |
pretty confident, had a few days to think about it :) |
and well, it is 90 lines of code if you ignore the newSymbol and imports, so it is not like super complex |
Fixes #581
Basically it goes back to the "non-optimized" property definition that was at the beginning, since that one works well with inheritance, while the optimized version does not due to the reusage of variables for the instance, the prototype, the possible prototype of the prototype etc.
Either way I don't think it is a big deal since each class is patched only once, and only per class, not per instance.