Page MenuHomePhabricator

Require classes being mixed in to be plain objects
Closed, DeclinedPublic

Description

Using mixinClass with a class that is setup to inherit properties from another using prototypal inheritance (such as by using inheritClass) causes problems because mixinClass only copies the class being mixed in's own properties.

We should detect when non-plain objects are being mixed in and throw an error of some sort to prevent people from making this mistake.

Event Timeline

TrevorParscal assigned this task to Krinkle.
TrevorParscal raised the priority of this task from to Needs Triage.
TrevorParscal updated the task description. (Show Details)
TrevorParscal added a project: OOjs core.
TrevorParscal subscribed.
Krinkle triaged this task as Low priority.
Krinkle set Security to None.
Krinkle subscribed.

Change 348059 had a related patch set uploaded (by Esanders):
[mediawiki/extensions/VisualEditor@master] Tag mixin inheritance hacks with bug

https://gerrit.wikimedia.org/r/348059

Or we could try and actually support this, by manually going up the inheritance tree and mixing everything in top to bottom.

Change 348059 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] doc: Tag mixin inheritance hacks with bug

https://gerrit.wikimedia.org/r/348059

Or we could try and actually support this, by manually going up the inheritance tree and mixing everything in top to bottom.

I can't find where I mentioned this but we discussed this before. The feature was declined for two reasons:

  • A mixin is based on the idea of copying over properties at run-time without inheritance. Breaking that inheritance by flattening it seems non-deterministic. The mixin would initially set up live inheritance that is than broken at an arbitrary point in the future when it is applied.
  • The concept of composing mixins is already trivially achieved by simply using mixins instead of inheritance.
Does not work
class MixinFoo
 ...

class MixinFooBar
  inherit Foo // !

class QuuxFobo
  inherit Quux
  mixin MixinFooBar // mixes MixinFoobar but not MixinFoo
Works
class MixinFoo
 ...

class MixinFooBar
  mixin Foo // !

class QuuxFobo
  inherit Quux
  mixin MixinFooBar
Also works (multiple)
class MixinFoo
 ...

class MixinBar
 ...

class MixinFooBar
  mixin Foo // !
  mixin Bar // !

class QuuxFobo
  inherit Quux
  mixin MixinFooBar