Properties inside the descriptor.methods

[koresar]

In example implementation check this line: https://github.com/stampit-org/stamp-specification/blob/master/examples/compose.js#L59

As you can see anything (like string and plain objects) can get into the descriptor.methods.

let stamp = compose({ methods: { s: 'my string', o: {} } });
console.log(stamp().prototype.s); // prints "my string"
console.log(stamp().prototype.o); // prints "{}"

And I think this is good. We should leave it as is.

Then why I'm creating this issue? A while ago Eric Elliot was strongly against putting any kind of properties to the prototype, thus stampit filters non-methods out. Most likely he's chaged his mind.

What are other people's opinion? @troutowicz @sethlivingston @unstoppablecarl

[troutowicz]

I feel like non-functions should be filtered out, just as stampit currently does. The descriptor property, after all, is named methods and the spec makes it clear this property is for methods only. I'm all for being unopinoinated, but in this case we would simply be enforcing the spec.

[koresar]

The spec does not say "'descriptor.methods` must contain methods only" or alike.

[troutowicz]

Sure it does.

https://github.com/stampit-org/stamp-specification#the-stamp-descriptor

• methods - A set of methods that will be added to the object's delegate prototype.

[koresar]

True. Someone need to change the example implementation.

[troutowicz]

Agreed, the tests would need updating as well.

[ericelliott]

In almost all cases, putting data on the delegate prototype is an anti-pattern. However, I have used the prototype for data in a few ways which I believe are totally valid. Not sure what to do. =)

[koresar]

My rule of thumb: give more freedom, people will understand themselves what's best for them.

[troutowicz]

I'm not against the idea of leaving this choice to the user, we should give as much freedom as possible. The method property definition in the spec would just need to be changed a bit.

[unstoppablecarl]

I have never run into a situation where I needed to put non-methods on the prototype. I have done it, but I have never NEEDED to do it. What is the case where you NEED non-methods on the prototype? The option should probably be there though.

If methods is not limited to methods it should be called something else. When I first looked at stampit and saw Object.create(methods) I thought it was strange that the prototype was named methods. It later made more (but not perfect) sense when I discovered non-methods were filtered out.

Maybe we should just call it what it is: prototype. Not a reserved word in js but should probably be something different like instanceProtoype maybe a compromise prototypeMethods to communicate you primarily put methods in it?

In almost all cases, putting data on the delegate prototype is an anti-pattern. However, I have used the prototype for data in a few ways which I believe are totally valid. Not sure what to do. =)

We should make this clear in the docs. Either explaining why non-methods are filtered out or that you should only use methods and give specific cases you would need non-methods on the prototype.

[koresar]

Appreciate your opinion @unstoppablecarl.

There is no best solution to this. We are leaving the "methods" name as is for now. And we are not introducing function filtering to it too, leaving the freedom of hacking with "methods" object.

We should come back to that question after we collect more usage statistics.