stampit-org / stampit

OOP is better with stamps: Composable object factories.
https://stampit.js.org
MIT License
3.02k stars 102 forks source link

Why aren't methods merged by extending the instance's prototype chain? #313

Closed eggyal closed 7 years ago

eggyal commented 7 years ago

This is less an issue and more a question/discussion over the design/specification. Apologies if this is not the right forum.

In https://github.com/stampit-org/stampit/issues/257#issuecomment-253190343, @koresar says:

Stamps do not allow prototype chaining. By design. Deliberately.

Why is this? I see that the prototype of stamped objects is their descriptor's methods, which is great—but why does not the merging of new methods, e.g. when composing stamps, not simply extend that prototype chain rather than merely create a new base prototype? That is:

import stampit from 'stampit';

const  A = { foo() { return 'A'; } },
      $A = stampit.methods(A),
      oA = $A(),

       B = { foo() { return 'B'; } },
      $B = A.methods(B),
      oB = $B();

A.foo = () => 'x';
B.foo = () => 'y';

// CURRENTLY (creating new prototype by merging method objects with Object.assign)
oA instanceof A; // false
oB instanceof A; // false
oB instanceof B; // false
oA.foo(); // 'A'
oB.foo(); // 'B'

// WITH PROTOTYPE CHAINING INSTEAD
oA instanceof A; // true
oB instanceof A; // true
oA instanceof B; // true
oA.foo(); // 'x'
oB.foo(); // 'y'

It seems to me that prototype chaining is far more powerful, expressive and useful. Why was the design decision taken not to use it?

koresar commented 7 years ago

hi there! Gonna list the reasons. Some might not be hard reasons, but combined they are a solid answer.

I hope I answered your question right. If not, feel free to ask.

eggyal commented 7 years ago

Hi @koresar, thank you for that answer. If I might just respond to each point in turn:

Stamps do use prototype, but only one level. No second or more level chaining.

Agreed, but this is the very point I was responding to. :)

Stamps consist not only of other stamps, but also stamp descriptors. These are POJO. Some of the descriptors contain methods property, some don't. I have no idea what the chain would look like in this case.

There could either be an object in the chain that contains no own properties or one could optimise that object out and directly link child prototype to parent.

Stamps are composed of multiple other stamps (and descriptors). So, a single stamp can have a dozen of "parent" stamps. I have no idea what the chain would looks like in this case.

Yet the incumbent merge operation effectively linearises such compositions already. It's no different.

Vertical hierarchy of prototypes is hard to reason about when you have, say, 10 prototypes down the chain. That's one of the main reasons stampit was born.

I thought the driving issue was to provide multiple inheritance, though I readily admit I need to watch Eric's talk again. I'm not convinced a chain of 10 prototypes is any more difficult to reason about than a composition of 10 stamps?

Performance wise it is always better to have only one prototype in an object chain.

This is a fair point, and one could certainly give developers the choice of inheritance methodology so that merge can be preferred over prototype chaining where performance is an issue.

You might want to read this discussion on instanceof implementation, and the referenced issues.

Thanks, will do.

And last, but not list. In production my stamps are composed of 20 other mini stamps.

As above, the incumbent merge operation is effectively linearising your composition already.

koresar commented 7 years ago

Thanks for the reply. Using the tree of stamps above could you please linearise them for me? Assuming, every stamp have a single method named same as the stamp.

I eager to see your linearisation algorithm. 👍

eggyal commented 7 years ago

@koresar: Surely it depends on the order of your existing composition?

koresar commented 7 years ago

@eggyal the tree is the order. For example,

Hopefully, I'm clear enough.

eggyal commented 7 years ago

But I mean, how are siblings currently composed? Isn't stampit(a,b,c) analogous to the prototype chain null <-- a <-- b <-- c?

koresar commented 7 years ago

@eggyal here is the reference implementation with comments: https://github.com/stampit-org/stamp-specification/blob/master/compose.js#L189

And here is the corresponding phrase from the specification:

methods are copied by assignment The regular Object.assign() is used.

trusktr commented 7 years ago

I eager to see your linearisation algorithm.

It is called class-factory mixins, as alternative to concatenation.

trusktr commented 7 years ago

ES6 let's you do

class Foo {...}
class Bar {...}
class Baz extends Bar {...}
class Blah {...}

class Lorem extends multiple(Foo, Baz, Blah) {...}

// or, depending on the prototype chain ordering that you want

class Lorem extends multiple(Baz, Foo, Blah) {...}

function multiple () {
  // ... I leave you to imagine the implementation ...
}

This is what Stampit seems to be purposed for, but it can also be done with ES6 classes and prototype chaining rather than with concatenation.

trusktr commented 7 years ago

Here's an example of multiple inheritance with class factory mixins in practice: https://github.com/trusktr/infamous/blob/master/src/motor/Sizeable.js#L26

ES6 classes are not limited by arthritic problems cause by single inheritance.

trusktr commented 7 years ago

As bonus, here's two multiple() implementations (with examples), but each one currently fails in certain more complex cases:

  1. https://gist.github.com/trusktr/05b9c763ac70d7086fe3a08c2c4fb4bf
  2. https://gist.github.com/trusktr/8c515f7bd7436e09a4baa7a63cd7cc37

The non-failing solution can be made with Proxy, but at the time of writing (and even still) not all engines have it in production, so I decided to come back to it later.

I don't really see a good enough reason to use Stampit when I can achieve the same with ES6 classes. Am I missing anything?

trusktr commented 7 years ago

Also note in my above linked example of the Sizeable class, the mixins can be used as classes.

F.e. , it could be

class Sizeable extends TreeNode {}
// or
class Sizeable extends TreeNode.mixin(Observable) {}
// or
class Sizeable extends TreeNode.mixin(Observable.mixin(AnyOtherClass)) {}

// and, in the last case

const s = new Sizeable
s instanceof TreeNode // true
s instanceof Observable​ // true
s instanceof AnyOtherClass // true
danielkcz commented 7 years ago

In my opinion, stamps are more about the ecosystem. You have some standardized descriptor that can be used to create stamp even without any supporting library since it's just POJO. Our monorepo is still at the beginning, but eventually, there could be many useful stamps that can be just composed together.

On the contrary, your solution with multiple lacks such opportunity as you cannot easily do these multiple inheritances without supporting library, can you?

trusktr commented 7 years ago

If you mean that the multiple() function is the supporting library, then yeah. It's easy to import with ES6 import/export. :)

In the very near future, it will be as simple as

import multiple from 'http://some-cdn.com/path/to/multiple/vXXX'

Stampit is also library support. Object.assign is also built-in library support.

Having to import something or even copy/paste it is not really a huge problem!

danielkcz commented 7 years ago

Having to import something or even copy/paste it is not really a huge problem!

I did not really try to imply it's a problem, but doesn't that mean you will be creating another kind of ecosystem similar to stamps? What is the benefit then? If classes would support multiple inheritances out of box then hell yea, it would be awesome, but they don't...

Also, it seems to me that classes can be rather limited in many ways and often you have to end up with various hacks (I personally consider Proxy a hack kinda). For example look at our privatize stamp. You cannot really do something that easy with classes if I am not mistaken.

And I am not sure about inheriting static properties/methods on classes. I did not dive into your solution, but can you like mixin together classes along with its statics? There is of course question of the need and if it's antipattern or not, but with stamps it definitely feels more natural and wanted.

koresar commented 7 years ago

Hi @trusktr

Thank you very much for that deep insight into the awesome ES usage.

You reminded me that there are already a bunch of ways to solve the "class inheritance" problem.

There is no clear winner. Every approach has pros and cons.

Since I'm using stamps a lot I'm going to list Pros of the stampit project:

A plan for the future: create the fromClass utility method which converts a class to a stamp. :)

Have a great coding, everyone. It's great to see so many ideas about object composition! The more we try - the more we learns as a community. 👍

ericelliott commented 7 years ago

I disabled instanceof in the design of stamps intentionally because instanceof lies across execution contexts, and when dynamic changes are made. You should not rely on it. Users of your factories and classes should not rely on it. A working instanceof is a liability, not a feature.

On that note, ES6 introduced a way to override broken instanceof behavior. It seems that using it disables a bunch of JS optimizations, though. Maybe in the future, that will change. For now, instanceof is a broken feature that should be avoided, not encouraged.

class composition using higher-order classes is cool, and if we can work out the problems with constructors and instanceof, maybe someday it will be a viable alternative to stamps. There are also lots of composition edge cases that users of class composition have not thought of that are solved problems in the stamp ecosystem. Here's just one example that stamps are immune to, because they're already factories:

Currently, switching from constructors to factories is a breaking change, and there are many reasons you might want to do so, including the need for dynamic abstract factories, object pools, proxies, etc... The change is so common, it's included a refactoring catalog in the book, "Refactoring" by Martin Fowler, Kent Beck, John Brant, William Opdyke, and Don Roberts.

That refactoring is not safe to do in JavaScript if you allow callers to use instanceof, or the new keyword.

As long as those problems exist, stamps should not enable instanceof, or encourage new in any way.

koresar commented 7 years ago

Closing as "by design" and "now an issue" kind of :)