Controlling the Stamp Composition (Proposal)

[koresar]

Original idea: Christopher
(IIRC I was talking about it somewhere sometime too.)

Carl's proposal: stampit-org/stampit#188 (comment)

Add .initStamp() method to stamps. It behaves exactly the same as .init() as far as setting, storing, and composing goes (internally stored as an array of functions, these arrays are concatenated when composing stamps). When a stamp is composed / created, after all other composition behavior (including merging the initStamp array), each function in the initStamp array is called on the resulting composed stamp. Each function is expected to return the same or new stamp to be passed to the next function in the array etc. No intermediate stamp objects are created maintaining non-enumerable properties on the stamp.

I use stamps. Very often (literally very) there is the need to debug/analyze a stamp's composition process/chain/order. See full list here.

Also, this will allow us to implement the last feature Traits have but stamp doesn't. It can be done in a flexible fashion - as a composable stamp/behavior.

In addition to extending the specification we should also develop few utility stamps which help with debugging/analysis of a stamp.

• Track the list of stamps composed - trackComposition stamp
• Track/Protect/Reject a property/method overlapping - trackOverlap stamp
• Find duplicate stamp composition - trackDuplication stamp
• etc

Sample code:

const logComposition = compose({ initStamp({left, right, result}) { // three descriptors
  console.log('LEFT', left.properties);
  console.log('RIGHT', right.properties);
  console.log('RESULT', result.properties);
}});

const A = { properties: { a: 1 } };
const B = { properties: { b: 2 } };
const C = { properties: { c: 3 } };
logComposition.compose(A, B, C);
// Will print:
// LEFT { a: 1 }
// RIGHT { b: 2 }
// RESULT { a: 1, b: 2 }
// LEFT { a: 1, b: 2 }
// RIGHT { c: 3 }
// RESULT { a: 1, b: 2, c: 3 }

[unstoppablecarl]

I have a working proposed solution here. https://github.com/stampit-org/stampit/tree/composers

What are your thoughts?

[ericelliott]

I'm open to this. =) PR?

[unstoppablecarl]

I thought there would be more discussion :D .

Do you think it is ready for a PR?

[ericelliott]

Seems pretty straightforward. I think a PR is a good next step. If we need more discussion, we can certainly discuss the PR. =)

[koresar]

The https://github.com/stampit-org/stampit/tree/composers branch is a completely different approach to the proposal above.

The proposal says about initStamp({left, right, result}) function.
Whereas, the branch is implementing the object instead.

var staticQueueComposer = {
        // replaces base composer
        name: 'base',
        order: 0,
        compose: function (self, other) {

• The "order" is not necessary and error prone (just think of how many bugs you fixed related to the CSS z-order). Take promises for example. The promise queue does not need that "order the execution". A developer just simply chains them the way he want.
• The "name" is not necessary too.

In other words, it's overcomplicated IMO.

Although, I like the idea of replacing the composition logic itself. Let's think of a simpler solution.

[koresar]

I might confuse something, but here is one more note.

Quoting Eric

Functional mixins force you to chose which bits are the "base class" and which bits are "mixins".

In the new implementation I don't what to choose which composers are "base" composers.

[unstoppablecarl]

This is intended to cover much more than the initStamp() case described above.

This is intended to provide the flexibility to accommodate the stamp composition functionality cases we have seen requested and have discussed on gitter. After spending a fair amount of time playing with different ways to do this I quickly found the problem is not simple or similar enough to work the same as .init(). This is more an inversion of control problem than a simple init function list.

I agree with the functional mixin comment but it does not apply. A stamp does have an essential 'base' part: composability. It will always have the base / default compose function. That is what makes it a stamp. I should have provided more documentation with my branch.

Reasons for named composer feature:

Prevent duplication

My first version did not have the name feature. With every call to compose() it would duplicate all composers to the new stamp (including the default 'base' composer). I then added a unique check and immediately realized equivalent composer objects will not always be ===. A simple name seemed like a reasonable way to identify composers and determine equivalency. Also a user may want to be able to have multiple instances of the same composer multiple times.

Composers on the source stamp replace composers with the same name on the target stamp. Preventing duplication where not desired as is the case with the base composer.

Prevent Double Execution

You have a composer that is intended to only be run once (such as the base composer). You then compose 2 stamps that both already have that composer, it will be executed 2 times. You would need to add a way to identify the duplicate and decide which one will execute and which one will skip within the composer itself. I tried to do this and realized nothing should be designed to work this way 😣 .

Allow replacement

When removing composers with the same name I decided that it should be "last in wins" to be consistent. If you ever needed to replace a composer when composing a specific stamp, you would need a way to identify the the stamp to replace, remove it, then add the new one within your composer. Setting them to have the same name seemed like a much simpler way to do that. This also allows the ability to completely replace the base composer with your own.

Debuging

I found I needed to identify specific composers anyway to follow and debug how things worked.

The feature is opt in (optional)

Composers do not have to be named. You can have anonymous composers that will never collide.

Reasons for the composer execution order functionality

The base composer is order: 0. Setting a composer to a negative order value ensures it executes before and a positive number after.

Execute before or after the default (base) composer behavior

With every composer use case I have seen it is essential that it is executed before or after the default compose behavior (usually before). If your composer executes after, there is no way to access properties on fixed before they are replaced. This could be worked around by copying the original but that is itself a compose operation and somewhat expensive n+1 processing that may not even be needed for composition (this copying would also be needed for the {left, right, result} proposal above as the left is mutated to become the result in the current code at least). The static queue is a perfect example of this problem. I considered having 'beforeComposers' and 'afterComposers' but realized an execution order would work just as well and allow someone to easily fix their execution order problems if they came up.

Order added in code !== desired execution order

I do not think you should be limited to determining the composer execution order by the order it is added in code.

Also opt in (mostly)

You do not really need to manage the order if you do not want to. You really just need to set a composer to be before or after the base / default functionality (-1 or 1).

Notes

I think anyone that digs in and starts to write a working solution to the cases we have seen discussed will quickly run into the problems I have described. I agree this is not a simple elegant solution, but I do not think there is one that will allow full control.

Here is my list of requirements for custom composition functionality to cover all the cases I have encountered or discussed.

• Composition behavior is retained and composed when a stamp is composed into a new stamp
• Composition behavior will be executed from the target and source stamp being composed
• Execute before OR after default composition behavior (bonus points if you can control specific execution order)
• Optionally replace default composition behavior (bonus points for exposing internal compose functions to make this easier)
• Optionally avoid double execution
• Optionally prevent duplication
• Ability to replace a specific composer on target stamp with a specific composer on source stamp

We should probably agree on a detailed description of requirements for this functionality so that we are all on the same page.

[ericelliott]

This is beginning to strike me as an over-engineered solution. Before we proceed further, let's back up and clarify some things:

1. What is the problem being solved? (Simple, specific, clearly understandable problem statement)
2. What are the requirements to solve the problem?
3. What are three specific use-cases we will test to ensure the problem has been solved?

When answering these questions, answer with simplicity and clarity in mind, in a way that would be fairly easy for a stamp newbie to grasp reading a stamp README or getting started guide.

If we can't explain the problem and the solution clearly to non-experts, it's not ready to be added to the spec.

It seems like you are focusing use-cases on collision mitigation. I've said it before and I'll say it again, collision mitigation is not required in the stamp specification because it can be handled at the stamp composition step, essentially any of the following techniques, without adding API overhead to the spec:

• The composite pattern: Give the resulting object a reference to composed pieces which may collide. The resulting stamp could expose the colliding keys with different names, or elect to expose one or the other using the original key name.
• Rename keys: Keys can easily be renamed during composition and composition time.
• Override: Intentionally replace one key with another, as with the defaults/overrides pattern.

These are all available design-time solutions that do not require any additional code. I would rather see some clear examples of each of these approaches than additions to the spec, if that is an option.

@koresar Has mentioned the desire to be able to track and maintain essentially a composition log: What happened to each of the source properties as composition occurred. I agree that it might be a nice debugging tool, but I'd like to emphasize that if you feel you need that, I suggest that you may be over-using stamps, or over-engineering your solutions. When it comes to object-oriented design, Keep it Stupid Simple should be your mantra.

That means flat inheritance hierarchies (you should not have stamps that inherit from stamps that inherit from stamps) , small component parts (which are unlikely to collide in the first place), relatively few components per stamp, etc...

Ask yourself very seriously: Am I keeping it stupid simple? Is there a chance we could solve these problems in simpler ways?

[koresar]

Ok guys. I'm fully on both of your sides.

@unstoppablecarl described my thoughts exactly as I saw them a year ago. I fully agree that we need that kind of advanced feature set.

Same time @ericelliott is 100% right too.

My proposal:

• Implement and use that feature in stampit only.
• Leave these specs untouched yet.
• Revisit the specs later as we get more info on real usability of the feature.
  • Maybe implement the feature in the specs too, maybe not. 🤷

Agree?

[unstoppablecarl]

I have not been paying as close attention to stampit development lately I didn't realize there was a 3.0 branch yet.

I was working on a project and pulled down the latest build (master) and could not get it to concatenate a static array on composition. I saw that a proposal was still being discussed in this issue and played with some solutions.

I wanted to better understand the problem by working out a prototype solution. I then set out to create a solution that would allow you to do anything you may need when adding custom composition behavior. This is probably too much and the scope should be more limited. This is why I was surprised when there was little discussion at first.

My personal use case

I am working on a game project that uses real time communication see https://github.com/primus/primus. Messages containing json are sent back and forth between client and server. I want a way to define a function on a stamp that converts its relevant data from or to json to communicate between the client or server. I need this behavior converting to or from json to be composable so that when I compose 2 stamps with defined to / from json behavior it is merged.

The code I tried to get to do this:

var Jsonable = stampit()
    .init(function(settings){
        var stamp = settings.stamp;
        this.toJSON = function(){
            return stamp.toJSON(this);
        };
    })
    .static({
        // converter functions
        _toJsonConverters: [], // needs to be concatenated on compose
        _fromJsonConverters: [], // needs to be concatenated on compose

        // add a to json converter function
        addToJsonConverter: function(fn){
            this._toJsonConverters.push(fn);
            return this;
        },

        // convert object instance to json
        toJSON: function(obj){
            return this._toJsonConverters.reduce(function(json, func, index, array){
                // return new or mutate json object
                return func(obj, json) || json;
            }, {});
        },

        // add a from json converter function
        addFromJsonConverter: function(fn){
            this._fromJsonConverters.push(fn);
            return this;
        },

        // prepares options object before calling factory
        fromJSON: function(json){
            var preparedOptions = this._fromJsonConverters.reduce(function(options, func, index, array){
                // return new or mutate options object
                return func(options, json) || options;
            }, {});

            return this(preparedOptions);
        },
    });


var StampA = stampit({
        refs: {
            foo: 'bar',
        },
    })
    .compose(Jsonable)
    .addToJsonConverter(function(obj, json){
        json.foo = obj.foo;
        return json;
    })
    .addFromJsonConverter(function(options, json){
        options.foo = json.foo;
        return options;
    });

var StampB = stampit({
        refs: {
            stampA: null,
            id: 99,
        }
    })
    .compose(Jsonable)
    .addToJsonConverter(function(obj, json) {
        json.stampA = obj.stampA.toJson();
        json.id = obj.id;
        return json;
    })
    .addFromJsonConverter(function(options, json){
        options.stampA = StampA.fromJson(json.stampA)
        options.id = json.id;
        return options;
    });

After thinking about it for a while every case I can think of or that we have seen so far could be solved by static property concatenation instead of replacement. Maybe we should just implement a way to do that?

[unstoppablecarl]

I just thought of one solution similar to the original initStamp proposal:

Add descriptor.staticInitializers. It is composed exactly the same as descriptor.initializers using array concatenation. Each function is called here
https://github.com/stampit-org/stampit/blob/v3_0/src/compose.js#L41. The function is passed a single argument: the stamp which it can mutate or return a replacement for.

This would allow you to have composable static initialization but would not give @koresar the debugging ability he wants.

[koresar]

@unstoppablecarl there is the new .configuration feature for complex cases like yours. The configuration object is deeply merged on compose.

Let me rewrite your case using the new "composables" syntax.

const Jsonable = compose({
  static: {
    addToJsonConverter(func) {
      const index = _.keys(this.compose.configuration.toJSONConverters).length;
      return this.compose({ configuration: { toJSONConverters: { `$(index)`: func } } });
    },
    addFromJsonConverter(func) {
      const index = _.keys(this.compose.configuration.fromJSONConverters).length;
      return this.compose({ configuration: { fromJSONConverters: { `$(index)`: func } } });
    }
  },
  initializers(opts, {stamp}) {
    const toJSONConverters = _.values(stamp.compose.configuration.toJSONConverters);
    this.toJSON = function () { /* using the toJSONConverters here */ };

    const fromJSONConverters = _.values(stamp.compose.configuration.fromJSONConverters);
    this.fromJSON = function () { /* using the fromJSONConverters here */ };
  }
});

const StampA = compose({ refs: WHATEVER })
.compose(Jsonable)
.addToJsonConverter(WHATEVER)
.addFromJsonConverter(WHATEVER);

The code above does not use stampit v3 or else.

[koresar]

Alright, here is an alternative solution which requires only a tiny change - one line to be moved 3 lines up.

Allow users to override stamp.compose method with statics

import compose from 'a-compose-implementation';

const LoggingCompose = compose({static: {compose(){ // override "compose" method
  console.log(arguments);
  return compose.apply(null, arguments);
}}});

compose(LoggingCompose, compose()) // not yet overridden
.compose() // overridden compose function
.compose({ methods: { F(){} }, properties: { a: 1 } }); // overridden compose function
// will log twice:
// 
// {}
//
// { '0': { compose: [Function: compose] },
//  '1': { methods: { F: [Function: F] }, properties: { a: 1 } } }

That's a simple and elegant solution IMO.

P.S. I'm pumped currently. Hard to control emotions, but I'm pretty sure that's simply the best way to go. It solves EVERYTHING we discussed above

[unstoppablecarl]

@koresar

Thanks for writing the example. The fromJson method would ideally be static so you do not need an instance to create an instance from json. It looks like it would work if it were moved to static correct me if I am wrong about that.

I am not sure I completely understand how the configuration object works. What does it do differently from staticProperties ? Wouldn't your code have worked the same using staticProperties instead?

This is a lot of trouble just to avoid having to concat an array though. I think having a staticInitializers would be more reasonable than directing users to writing this kind of code to emulate array concatenation.

The code above does not use stampit v3 or else.

what? is that a threat 😕

I am not sure overriding stamp.compose solves EVERYTHING. Correct me if I am wrong: It only works when calling the chained 'compose' method from the stamp. It does not allow composition of multiple compose behaviors. It does not cover the static initialization case as it is only overridden when calling the chained stamp.compose method. It does somewhat cover your debug case if you stick to using the chained compose function. I think we need something that the default compose behavior is aware of like staticInitializers.

[koresar]

It looks like it would work if it were moved to static correct me if I am wrong about that.

¯\_(ツ)_/¯ But that's not he point I wanted to make with my example.

Wouldn't your code have worked the same using staticProperties instead?

Yes, it would, but using deepStaticProperties. I just didn't want to expose the list of functions (your code exposes 2 "private" static properties). The configuration kind of hides it (almost :) ). But that's also not he point I wanted to make with my example.

I think having a staticInitializers would be more reasonable than directing users to writing this kind of code to emulate array concatenation.

My proposal to allow overriding the .compose function can replace staticInitializers easily. Allowing the wider possibilities, needing almost no changes to the implementation(s).

It does not cover the static initialization case as it is only overridden when calling the stamp.compose.

I think I'm missing something. Could you please point why do you think so? (I'm really confused, it's 1 am after all. :) )
If you can, please provide code. It's easier to talk for me. Thanks!

P.S. Really appreciate your opinion @unstoppablecarl

[unstoppablecarl]

@koresar

¯_(ツ)_/¯ But that's not he point I wanted to make with my example.
What is the point? I am clearly not understanding.

What exactly is the stamp.compose.configuration object for then? In the code it appears to be treated a little differently from staticProperties https://github.com/stampit-org/stamp-specification/blob/master/examples/compose.js#L55 . Is this doing something special that I am not seeing?

If you can, please provide code. It's easier to talk for me. Thanks!

As you show in the comments the overridden compose function is only called when directly chained from the stamp. The following cases will not call the overridden compose function:

import compose from 'a-compose-implementation';

const LoggingCompose = compose({static: {compose(){ // override "compose" method
  console.log(arguments);
  return compose.apply(null, arguments);
}}}); // custom compose not called when LoggingCompose is created 

compose(LoggingCompose, compose()) // custom compose not called

LoggingCompose
  .compose() // overridden compose function IS called surprising and confusing everyone :P

compose(compose(), LoggingCompose); // then later not called

I think it is pretty clear that this does not accomplish what we need.

I'm going to try implementing staticInitializers in example composer to better communicate it.

[koresar]

Is this doing something special that I am not seeing?

You're seeing it all. IF the role of configuration is vague could you please point in the README the passage about it which was not clear. Maybe we should add more info on it.

I think it is pretty clear that this does not accomplish what we need.

Good point... crap. :) I need to think.

[unstoppablecarl]

Well in the readme heading "Configuration Example" it describes behavior that is not in the example code and links to an empty repo. So no it is not clear what it is for. It is just for the collision example?

[koresar]

Yes, that's just an example. Not sure why it exists. :)

The essence of configuration is described here: https://github.com/stampit-org/stamp-specification#the-stamp-descriptor

[unstoppablecarl]

Ah I get it now. At first I thought functionally configuration was identical to deepStaticProperties. I see now that configuration is Not merged into the stamp and deepStaticProperties is.

[unstoppablecarl]

After discussing with @koresar

The only complete way to intercept all calls to compose() is to override it with a debug implementation that meets the spec but also internally logs the left right result (target source result) objects.

It is not reasonable to have the left right result (target source result) built into the spec as you would need to make a copy of the target stamp before it is mutated by composing the source stamp into it yielding the result stamp. This would be simple to do within the overriding debug compose() implementation.

Stampit should have an api like stampit.compose and always reference stampit.compose when composing so that it may be overridden in the same way.

[unstoppablecarl]

moved concatenation discussion to #66

[ericelliott]

Override merged.

[koresar]

For the future reference this was implemented in #71