Different behavior for method collision

[anticore]

Greetings, stamp people.

I've been messing around with stamps for a relatively small project and it has been a fantastic development experience.

That said, I'm interested on how to implement a different behavior for method collision.

As far as I understand, collision defer calls the colliding methods sequentially. What I would like to be able to accomplish is to pipe the return value of each colliding method to the next.

For example, if I wish to have a 'stamp-wide' toJSON() serializer, I would like each of the composing stamps to have a toJSON() method. If the toJSON() methods were expecting a previous value, that would considerably simplify the complexity of such a feature.

Something like:

const Stamp1 = stampit({
    props: {
        name: "stamp1"
    },

    methods: {
        toJSON(previous) {
            const _json = { name: this.name };
            return !previous ? _json : {...previous, _json};
       }
});

Does this make sense? Is there an already established way to accomplish something like this?

Thanks in advance.

[koresar]

Wow. Such a great idea.

Few options here.

1. Copy paste the collision stamp to your code. Adapt it to your need.
2. Wait for me to implement this feature. Btw, how do you see the API of the feature? Please provide some code examples.

Cheers

On Tue., 22 May 2018, 02:43 João Faria, notifications@github.com wrote:

Greetings, stamp people.

I've been messing around with stamps for a relatively small project and it has been a fantastic development experience.

That said, I'm interested on how to implement a different behavior for method collision.

As far as I understand, collision defer calls the colliding methods sequentially. What I would like to be able to accomplish is to pipe the return value of each colliding method to the next.

For example, if I wish to have a 'stamp-wide' toJSON() serializer, I would like each of the composing stamps to have a toJSON() method. If the toJSON() methods were expecting a previous value, that would considerably simplify the complexity of such a feature.

Something like:

const Stamp1 = stampit({ props: { name: "stamp1" },

methods: {
    toJSON(previous) {
        const _json = { name: this.name };
        return !previous ? _json : {...previous, _json};
   }

});

Does this make sense? Is there an already established way to accomplish something like this?

Thanks in advance.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/stampit-org/stamp/issues/45, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjCLx3uYHR1QNtcIvLOI2QwhLfDhMyoks5t0u6zgaJpZM4UHSGf .

[anticore]

@koresar I'm not quite sure... Is it good practice for the stamps to be 'aware' they'll be used with a collision stamp? If so I don't see a problem with explicitly declaring the methods with a first argument to expect the return value of any previous colliding methods (see example above). I'm not sure how to handle 'dumb' stamps though.

I might take a crack at adapting the collision stamp, but I'm looking forward to see what you can come up with.

[koresar]

To fix the 'awareness' concern you can "inherit" (aka compose with) from your collision stamp.

const DeferToJSON = Collision.collisionSetup({ defer: ["toJSON"] });

const Stamp1 = stampit(DeferToJSON, {
  props: ...
  methods: {
    toJSON() { ...

Now the stamp is aware it has a deferred "toJSON" method.

---

Semi good news. I have just published @stamp/collision v1.1.0. Now if you "defer" a method it will return an array of method returns.

const MegaStamp = stampit(Stamp1, Stamp2, Stamp3, Stamp4); // each stamp have "toJSON" method

const results = MegaStamp().toJSON(); // array of 4 items
const json = Object.assign({}, ...results);

That's not exactly what you need, but it works today.

---

The functionality you ask is called "pipeline". It's not as quick to implement as I thought. I need some more time.

[anticore]

@koresar That definitely works for now. Thank you for your quick response and implementation. Interested to see how you solve the pipeline problem.

[koresar]

Taking a second look I realise that API of this stamp could be better. More flexible and more featureful. For example, I always wanted to protect props and static methods collision - not just methods.

How about this API:

import {collisionSetup} from '@stamp/collision';

const PipeToJSON = collisionSetup({ methods: { toJSON: "pipe" } });
const DeferToJSON = collisionSetup({ methods: { toJSON: "defer" } });
const ForbidCollisionToJSON = collisionSetup({ methods: { toJSON: "forbid" } });

const ForbidCollisionPropFoo = collisionSetup({ properties: { foo: "forbid" } });

[anticore]

Looks good.

Can you see properties being able to use the pipe collision handler? For example, arrays being concatenated, objects deep-merged, etc. Not sure about primitives.

[koresar]

I do not see regular properties piped or deferred. We have deepProperties for array concatenation and object deep merging. https://stampit.js.org/api/deep-properties

[koresar]

I'm working on it. Rewrote the whole thing 3 times now. Found a best solution. It's coming

[koresar]

I'm close :)

[koresar]

The PR is ready: #48

[sammys]

Hi there stampers. Since this is my first post to @stamp I'd like to thank everyone for making this suite of packages! Utterly fantastic! Let's get down to business hehe

I've started working on a project using stamps and several of the packages require some rather fancy collision handling. I ended up almost completing a reimplementation of @stamp/collision and then stumbled upon this issue. What I implemented might be interesting to others so I thought I ought to mention what I did.

First up, I felt defer wasn't really describing what was going on so I renamed it to map so it's more obvious what it does. I also implemented reduce, which is equivalent to pipe as described in this issue.

Second of all, I implemented async versions of both. I'm heavily using reduceAsync in my packages to properly handle asynchronous initializers though that was via an additional setting initializersReduceAsync. Even though that is working well I think the new implementation in #48 is a better solution and, as you might imagine, I'm rather glad I stumbled on to this issue.

So, I'm about to start merging what's been done in #48 and what I've done with naming and async support so I can get the full awesomeness for my project. Are there any concerns about the naming or anything else that anyone would like to discuss and is there any interest in having it as a PR?

[sammys]

Update: After merging what I have with the implementation in master all tests are passing for the map setting (formally known as defer) for methods using this type of syntax:

{ methods: { map: ['draw'] } }

I've decided to use the following names for the settings:

• map: returns an array of values
• reduce: returns a single value and whatever is bound to this will be the starting value for reduce
• mapAsync: returns an array of Promise
• reduceAsync: returns a single Promise and whatever is bound to this will be the starting value for reduce

In #48 there was use of the word aggregate so that's what I've used to designate the above settings

I also had to tweak compose a little bit so it would play better with the strict index signature requirements you have in place

[koresar]

Hi @sammys

I'l reading every word you write :) Thanks for the work!

I myself couldn't finish the coding of the feature. Maybe you could? I can give you write access to this repo. Do you need it?

I'll be happy to accept any design/update you feel is better. So, yeah, a PR would be great. :)

[sammys]

Hi @koresar ... I appreciate the kind words. I'd be happy to contribute some time and push stamps forward a little more. Perhaps we can discuss details somewhere else since that is OT?

Update: I've completed implementation of all four aggregation types (map, reduce, mapAsync and reduceAsync) for methods. I fixed what I consider to be a bug (or limitation) in the current released implementation also.

The released implementation requires that every subsequent composition containing a Collision-setup stamp requires the Collision to be setup again in the new composition. I believe the original intent of this stamp was that the Collision setup survives future compositions so I fixed it. Please let me know if this was not the original intent.

I have to add a few more tests and then I'll push a branch to my fork for anyone wanting to weigh in on the changes.

[koresar]

You can reach me in twitter DMs if you believe that's a better communication channel.

I'm waiting for you to tell me what to do. :)

[sammys]

Both the implementation and tests were written in Typescript. I've migrated tests to JS and run jest with the above result. Now that I've confirmed that the migration is sound I'll push it up to a new branch on my fork.

[koresar]

I hope the TS would stay as the primary language.

On Wed, 31 Mar 2021, 20:30 Sammy Spets, @.***> wrote:

[image: Screen Shot 2021-03-31 at 16 23 35] https://user-images.githubusercontent.com/1011469/113122314-b3ac3580-923d-11eb-98e0-1c467bf73edf.png

Both the implementation and tests were written in Typescript. I've migrated them to JS and run jest with the above result. Now that I've confirmed that the migration is sound I'll push it up to a new branch on my fork and add an update to this post once it's done.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stampit-org/stamp/issues/45#issuecomment-810921567, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMMEL4VV7LT37YSD3MAVYDTGLTSXANCNFSM4FA5EGPQ .

[sammys]

I hope the TS would stay as the primary language. …

Yeah. Typo. I migrated the tests to JS. Updated the post.

[sammys]

Let me know what you think. I haven't made any changes to README.md yet awaiting approval of this direction and any honing that is to be made before it is worthy of master.

[sammys]

Now that we have the new settings format available I've thought of one other way we can expand the functionality of @stamp/collision a little more once #80 is merged. We could introduce a way to force particular aggregated methods to be executed either before or after the others. Here's an example to illustrate:

Let's say you have stamps A and B with each having an initializer (initA and initB). They are implemented in different third-party packages so neither are aware of the other in their code.

Along comes Johnny A. Developer and composes stamp D1 with A and stamp D2 with B and then they all get composed into stamp BIG: compose(compose(D1, A), compose(D2, B)).

Let's now put in a requirement that just the initializers initA and initB have to run after all other initializers in the BIG stamp or any other stamp that BIG is composed into.

To make this happen we could add a setting in the initializers domain. E.g. { initializers: { forceLast: [ 'initA', 'initB' ] } }

What this does is push initA to the end then push initB after that. Any thoughts?

[koresar]

I gave a deep thought to Promise returned from initialiser. Here are my thinking.

• Stamps are alternative to classes. One can't have async constructor in any of the languages.
• Stampit used to handle promises returned from initialisers. It was a mistake back then - code got significantly complex, users have never adopted the feature. We removed the promise support from stampit.

Sorry I'm coming too late. But I suggest to implement Promise support some day later in a separate package.

[koresar]

I'm looking at the PR and... Maaate, this is so neat! Loving the unit tests especially! So good. So perfect!

[sammys]

I gave a deep thought to Promise returned from initialiser. Here are my thinking.

• Stamps are alternative to classes. One can't have async constructor in any of the languages.
• Stampit used to handle promises returned from initialisers. It was a mistake back then - code got significantly complex, users have never adopted the feature. We removed the promise support from stampit.

In the project in which I've used Stamps, all initialisers are async because the Stamps (and any they are composed into) need to be capable of performing async processing during construction to simplify code. I implemented what you now see in the PR to support these. There were no terrible side-effects and also no complexity beyond what is normal for async code.

After seeing the simplification it enabled first hand I now believe the OOP limitation sucks and is something Stamps overcomes and thus gains yet another major advantage.

I'd love to hear what the problems were with async. Can you point me to sample code or a commit hash to checkout?

I understand that the stamp specification currently does not support a promise returned from an instance and I will remove the Promise<> type declaration. That said, the implementation in the PR still works