stampit-org / stamp-specification

The Stamp Specification: Composables
434 stars 16 forks source link

The "composers" proposal #109

Closed koresar closed 6 years ago

koresar commented 7 years ago

There are many issues we can solve with that proposal. But first let's list the issues.

1) What if I want to separate behaviors to be composable: one to collect all the stamps a final stamp is composed with, and another one to make sure there are no method name collisions.

Surely, both can be solved with the infected compose. But we cannot easily double infect. Second infection must know that first infection exists to make sure both of them work. Basically, infection is great when there is only one. The second infection typically overwrites (removes) first infection unintentionally.

So, if you want to have two infections - the collectStampsCompose and the detectOverridesCompose - you might get only one.

2) If an initializer overrides an object instance (e.g. with a function), then the next initializer(s) in the chain might not be ready for that. So, in that case I want to enforce my initializer to be last in the initializers array.

So, sometimes it is really needed to have an initializer either to be invoked the first or the last.

3) I found that typically when someone is using infected compose all she/he want is to track composition. Like, collect all the composables a stamp was created from. Infected compose is a difficult concept to understand. People are really struggling with it. We should provide a simple solution.

4) Also, I'm seeking for a way to replace "class and property decorators" ECMAScript proposal with a composable behavior (aka stamp). This would be a huge win towards a proposal to the TC39.

5) ... other composition-controlling logic.

You might have noticed, that the default metadata merging algorithm does not suffice the above needs. Each example requires the merging logic to act a little different. This little difference can be part of the stamp's metadata.

The proposal

Have one more metadata - composers. It should be an array of functions (just like the initializers). Stamp.compose.composers.

The composers should be invoked sequentially (one by one, just like the initializers) at the time the metadata composition happens.

The composers function examples (really not final, help me here please): 1)

/**
@param {Composable[]} composables - the list of composables which will be merged together.
@param {Stamp} stamp - the resulting stamp made of the `composables` array.
*/
function (composables, stamp) {
  return undefined; // means this composer does not change the resulting stamp
  return something; // means this composer replaces the resulting stamp
}

2)

/**
@param {Descriptor[]} descriptors - the list of descriptors which will be merged together.
@param {Descriptor} result - the resulting descriptor made of the `composables` array.
*/
function (descriptors, result) {
  return undefined; // means this composer does not change the resulting metadata
  return something; // means this composer replaces the resulting descriptor
}

3)

/**
@param {Descriptor} target - the new (resulting) descriptor
@param {Descriptor} descriptor - the descriptor which should be merged into the `target`
*/
function (target, descriptor) {
  return undefined; // means this composer does not change the resulting target
  return something; // means this composer replaces the resulting target
}

Currently, I like the number 3 as the most flexible and simple of all.

Ask your questions, please.

danielkcz commented 7 years ago

Well I do like general idea behind this for sure. I do not like name composables, but sadly I am not able to come up with something better yet.

Option no. 3 looks best as it can be easily abstracted if someone wants to use array. Other would need an array always.

I am not so sure about returning undefined really. If nothing has changed it should return original stamp in my opinion. I would even force to always return some value to avoid mistakes.

ericelliott commented 7 years ago

IMO, this spec is already too complicated. You're talking about complicating it even more. In my opinion, people who think they need this (and "infected" compose) are relying too much on stamps.

koresar commented 7 years ago

You are right, this is a complication. But there is a justification.

1) I don't want stamps to die like traits.js because of a stale development. I want to continue the research for better stamps. Show must go on. 2) That research includes implementing stamps for other languages. C#/Java/Swift/you name it. Thus, experimenting with all kind of features is necessary. 3) We can always simplify that back with specification v2. 4) None is forced to use the new stamp features. But every tool need to cater for "pro users" when it grows. 5) I really-really dream to replace the 'class and property decorators TC39 proposal' with composable stamps. I feel that they are trying to bring Java to JavaScript. Whereas I want to bring JavaScript to Java with stamps. And I can't. Because the stamps lack features.

@ericelliott it is good you have expressed your opinion. But please, avoid stopping our passion. Better facilitate it. :)

ericelliott commented 7 years ago

I'm concerned that too much complication will stop your passion. ;)

At least, it will hinder adoption and cause confusion. I'm already seeing people thinking infected compose is the bees knees, but as you mentioned, using infected compose is a bit like passing arbitrary multiple arguments to stamps -- it hinders composition... so now you have to add composers so that you can compose infected composers.

See where this is going? You introduce some complication to solve a perceived problem, which creates another problem, which forces you to introduce more complication to solve that problem...

This is a bit like taking a pill to help you stay awake, but then you stay awake for too long, so then you take a sleeping pill at night to help you get to sleep, and then you have trouble waking up in the morning, so you drink some coffee, but that gets your heart rate going too fast, so you take a relaxation pill, which makes you drowsy so you take the pill to stay awake...

koresar commented 7 years ago

Okay. I got your point about infected compose argument.

What do you think about 'class and property decorators' argument above? How about the C#/Java/Swift argument? And how about traits.js argument? And what about the "pro users" argument?

Do any of them worth the effort?

P.S. I know people who like the infected compose feature very much. :)

danielkcz commented 7 years ago

Honestly I had no need for reinfecting compose in any way so far. I am using plain stampit and I am more than fine with it. Maybe the projects weren't that much complicated so far yet or I am just trying to keep things simple, hard to say. I am sure I wouldn't use these composers anytime soon.

Btw, if you want to avoid stale development, you can refocus on react-stamp which is laying aside and slowly dying.

koresar commented 7 years ago

Okay. So now I'm going to quote every person who asks for that feature.

Here is another message I received today. The person is talking about the infected compose feature.

Ay ay ay, conceptually this is exactly what I want to be doing, but I cannot wrap my head around this. More investigation, and definitely a simpler API needed.

koresar commented 7 years ago

There was one guy in the Gitter chat. I couldn't find the exact quote. But basically, he couldn't get his head around infected compose. I must admit, it's a powerful bug hard to grasp feature.

So, for him this proposal would be very useful too.

UPD: Found it

danielkcz commented 7 years ago

If I am not mistaken, there are some most common patterns that people wants to solve with infected compose, right? Eg. some sort of collecting list of stamps or collision detection. Perhaps instead of trying to figure out universal solution, we might create couple of packages that actually solves these common use cases and then people can get inspiration there.

koresar commented 7 years ago

It will be very hard to to make two packages like that compatible. Moreover, the API of those will probably be even more complex than the proposal.

danielkcz commented 7 years ago

Well I am not saying to create some super robust solution for all use cases people need. Instead do some basics ones that are most common that people can install and use. I am not sure what these are. As I said, I had not need for these just yet. Those packages can then improve over time by adding more features/configuration based on what people actually need. Or they can fork and add their own stuff.

The actual "composers" proposal is not bad for sure, but I would say we just need more examples and uses cases before adding it.

koresar commented 7 years ago

That's a somewhat complicated approach as to me. We need more data, definitely.

JosephClay commented 7 years ago

I'm failing to see how collision is our problem to solve.

I've never had anyone need to detect collisions when using Object.assign or lodash/merge.

In classical inheritance, you need to know the properties of what you're inheriting from to avoid collision problems...all the way down the inheritance chain. Makes it a user-land issue.

ericelliott commented 7 years ago

Yep. I want to see several practical, useful examples. Basically, write clear documentation and use-case examples and run them through peer review before adding to the spec. This should be the process for adding anything to the spec. Essentially, fill out this template:

Simple Example

Motivation

Deeper Explanation

API Proposal

2 Additional Use Case Examples

koresar commented 7 years ago

Third example.

This fork can be implemented as a single cross compatible stamp/behavior using this proposal.

Without the proposal this can be implemented as an infected compose. But... as we already know, infected compose is hard to grasp, hard to have more than one of them in your code.

koresar commented 7 years ago

@FredyC

Well I am not saying to create some super robust solution for all use cases people need. Instead do some basics ones that are most common that people can install and use. I am not sure what these are.

First of all, if you are keen to develop and maintain all those packages - please do. Personally, I have 0 free time.

Second, I agree with @JosephClay - how come it is our problem to develop and maintain all those 'common case packages'?

And third, - I want to give people a simple API so that they can implement those 'common cases' themselves.

koresar commented 7 years ago

@ericelliott since you are directing us toward the TC39-like processes I'll quickly draft the new must-follow process sometime today. Do you think GitHub's wiki is an appropriate place for that or better create a new .md file like decision-making.md to this repo?

koresar commented 7 years ago

This could have been a simple stamp: https://github.com/parro-it/private-class

Although, it cannot be implemented consistently without the proposal. Because this logic of "hiding" the private variables must come the last of all initializers. But can be the first.

import PrivatizeUnderscoredProperties from 'private-stamp';

const MyStamp = stampit({ init() {
  this._name = 'Alhazred'; // assigning a private value in an initializer
}});

// PrivatizeUnderscoredProperties initializer is the FIRST
stampit.compose(PrivatizeUnderscoredProperties, MyStamp);

console.log( MyStamp()._name ); // "Alhazred" <- the property was NOT hidden
console.log( MyStamp().name ); // "undefined"

But with the proposal it would go smooth (the code is the same, except comments):

import PrivatizeUnderscoredProperties from 'private-stamp';

const MyStamp = stampit({ init() {
  this._name = 'Alhazred';
}});

// PrivatizeUnderscoredProperties initializer will be the LAST
stampit.compose(PrivatizeUnderscoredProperties, MyStamp);

console.log( MyStamp()._name ); // "undefined" <- the property WAS hidden properly
console.log( MyStamp().name() ); // "Alhazred"

The composers logic added by the PrivatizeUnderscoredProperties behavior/stamp would ensure that it's the last in the initializers list.

ericelliott commented 7 years ago

privatize() could be implemented as a higher order stamp without this proposal.

const privateStamp = privatize(compose(some, stamps, withPrivate, props));

I suspect the same could be said about many things you'd be tempted to do with this proposal -- with less complexity.

koresar commented 7 years ago

Eric, we can do everything with higher order functions. Not only that privatize, but also any other feature stamps have. Moreover, stamps are not needed at all. Why bother improving stamps if there is good old functional programming? :)

I'm somewhat disappointed in the fact we stopped evolving because many of you think "that's it! none must introduce any more complexity!"

Yes, many think that current specification of stamps is good enough. It is true. It is good enough for many your use cases. Also, you are afraid of complexity. This is a very valid point.

Meet Clarke's three laws. Especially the 2 and the 3:

  1. When a distinguished but elderly scientist states that something is possible, he is almost certainly right. When he states that something is impossible, he is very probably wrong.
  2. The only way of discovering the limits of the possible is to venture a little way past them into the impossible.
  3. Any sufficiently advanced technology is indistinguishable from magic.

I all want is to give people an ability to do seamless magic with their stamps and object instances under the hood. The complexity will be hidden inside the compose implementation.

koresar commented 7 years ago

As I'm chatting with many people I find that almost everyone likes the "infected compose" feature. People are using it more and more. It's a hot topic. We MUST evolve it.

Here is another example: gitter archive And the same request by @FredyC here: stampit issue In short - people want intializers to support promises.

That request can be implemented as a composable behavior (aka stamp) only after this proposal gets implemented.


So, the question to everyone: those 5 examples I've already brought across last 3 weeks are enough to consider this feature implemented or you need something else?

ericelliott commented 7 years ago

@koresar Can we see the full example for privatize written both ways?

import PrivatizeUnderscoredProperties from 'private-stamp';

const MyStamp = stampit({ init() {
  this._name = 'Alhazred';
}});

// PrivatizeUnderscoredProperties initializer will be the LAST
stampit.compose(PrivatizeUnderscoredProperties, MyStamp);

console.log( MyStamp()._name ); // "undefined" <- the property WAS hidden properly
console.log( MyStamp().name() ); // "Alhazred"

What does private-stamp look like?

vs

import privatize from 'privatize';

const MyStamp = stampit({ init() {
  this._name = 'Alhazred';
}});

// Using higher-order function instead of composable stamp.
const privateStamp = privatize(MyStamp);

console.log( MyStamp()._name ); // "undefined" <- the property WAS hidden properly
console.log( MyStamp().name() ); // "Alhazred"
danielkcz commented 7 years ago

I just would like to add that comment of mine is rather old and I no longer think it's a good idea to return Promise from initializer. It's kinda hard to track that since usual behavior is to get object instance. Suddenly there is a Promise because one stamp in a chain wanted it like that. I withdraw my proposition :)

koresar commented 7 years ago

That's how the 'private-stamp' module would look like using the API number 3:

import privatize from 'privatize';

const privatizeSelf = function () { return privatize(this); }

export default const PrivatizeUnderscoredProperties = compose({
  composers: [function (target, descriptor) {
    target.initializers = (target.initializers || []).filter(init => init != privatizeSelf); // remove 'em all
    target.initializers.push(privatizeSelf); // put it to the end
  }]
});
ericelliott commented 7 years ago

See, that's significantly more complicated. I'd have to look at docs and examples every time (because I'd use this very rarely).

Just so you can do everything with stamps? Stamps are by definition "composable factory functions". That's what they've always been. Trying to force this behavior into a stamp form gives you "composable factory functions or factory function modifiers that can do whoknowswtf" -- a significantly tougher description to swallow. It forces you to add overhead that isn't necessary just so you can stuff them in as compose arguments instead of wrapping a function around a stamp?

I'm just really not feeling the need or the benefit of this proposal.

Also, composers are significantly less flexible than a higher-order function. What if you just want to privatize props from one stamp, but leave others alone? With a higher-order function, that's trivial:

const myStamp = compose(privatize(stampToPrivatize), stampNotToPrivatize));
koresar commented 7 years ago

See, that's significantly more complicated.

How come MyStamp.compose(PrivatizeUnderscoredProperties) is more complex than the privatize(MyStamp); ? :)

Although, that last code example of your really make sense.

I'm just really feeling the need and the benefit of this proposal.

ericelliott commented 7 years ago

How come MyStamp.compose(PrivatizeUnderscoredProperties) is more complex than the privatize(MyStamp); ? :)

Because with the latter, you need to implement privatize and add it to composers, i.e, all of this is extra code:

const privatizeSelf = function () { return privatize(this); }

export default const PrivatizeUnderscoredProperties = compose({
  composers: [function (target, descriptor) {
    target.initializers = (target.initializers || []).filter(init => init != privatizeSelf); // remove 'em all
    target.initializers.push(privatizeSelf); // put it to the end
  }]
});
ericelliott commented 7 years ago

Granted, I suspect we're burying some details by omitting the privatize() HOF implementation.

koresar commented 7 years ago

How about that example:

const MyStampNamed = MyStamp.compose({ properties: {
  _name: 'unnamed',
  _somethingElse: whatever
}})
.compose(Privatizable)
.privatize('_name')

I'm pretty sure you can implement that using FP, but question is different. Do you find that useful?

ericelliott commented 7 years ago

That last example is already possible with the current spec, and the solution that I have suggested to others in the past.

koresar commented 7 years ago

Let's do the same way we did with most current features of Stamps. Let's implement it in stampit and see if it gets popular. If yes - then we add it to the specs.

Example: 1) Promisified intializers - only 2 people I know still need it. None else. Thus we dropped it. 2) Statics - half of the people using it. If not more. So, it's part of the specs now.

Sounds like a plan?

ericelliott commented 7 years ago

Sure, but maybe the docs should show some examples of how you can alter stamps if you want to using higher order functions instead of using composers. At least that way people won't think that's the only way to do things. =)

koresar commented 7 years ago

Great point. I'll create an issue in stampit reflecting that.

unstoppablecarl commented 7 years ago

I don't think it would be wise to add this to core functionality. I think many people would be surprised when underscore prefixes changed behavior.

koresar commented 7 years ago

Stampit v3.1 was just published. The composers feature is experimentally implemented. The composers array is stored in configuration.composers metadata.

hoschi commented 7 years ago

I'm updating stampit from v2 to v3 at the moment and like the idea of composers. The collisions package is something useful in my opinion and I added https://github.com/stampit-org/stamp/issues/17 as another use case.

koresar commented 6 years ago

Year later overview.

We can safely denote, that the feature is useful. Time has shown that people who opposed have found it useful now.

Going to put up a PR soon(ish).

koresar commented 6 years ago

PR is ready - #118

I will merge it in a week if no objections.