properties vs deepProperties priority about stamp-specification HOT 20 CLOSED

Ooh, good one. =)

Ideally this should happen:

1. 'deep'
2. 'shallow'

The reason is so that users will have an easy time of understanding precedents. Where there are any name conflicts, the last thing in should win, but this case clarifies that we really need to spell out the algorithm for the combined behavior of stamp creation and instance creation.

We have some undefined behavior here we need to lock down.

from stamp-specification.

in stampit v2 is was solved with mergeUnique.
https://github.com/stampit-org/stampit/blob/master/src/stampit.js#L123

from stamp-specification.

IMO, the outcome for the both have to be identical. It'd simlify things.

from stamp-specification.

That would simplify the implementation. Let's go with that... but we'll need to describe the algorithm for instance creation better in the spec.

from stamp-specification.

Agree. Simple implementation is a must-have IMO.

I am busy atm. Will come up with something later.

from stamp-specification.

There are three simple strategies only:

1. Shallows taking over deeps.
2. Deeps taking over shallows.
3. Deeps are merged into shallows but not override any existing properties.

The 3-rd is quite implicit and might lead to unexpected behaviours.

Or, take the most complicated fourth path - the ideal one Eric outlined above...

I'm largely confused at the moment.

from stamp-specification.

I think there is only one option:

1. Shallows taking over deeps.

All other either corrupt shallow props in stamp state, or lose the pass-by-ref property of shallows.

Note that pass-by-ref means non-plain objects work (dom-nodes, objects with methods, etc).
This really should not be broken.

Specifically I mean: during .compose() you follow order as usual, as each type has it's own path for merging, but on instance creation, deep props are installed first, then shallow ones.

from stamp-specification.

Here's another option: We could throw on cross-descriptor collisions. In other words, duplicate keys in methods would just override, but duplicate keys between methods and propertyDescriptors could throw at stamp creation time -- which would preserve the often desirable defaults/overrides behavior for the common cases, and let stamp authors deal with collisions at design-time for those edge cases.

We'll leave it to stamp authors to do whatever they want in initializers.

from stamp-specification.

1. Shallows taking over deeps.

Or throw at stamp creation and let the author decide what to do.

from stamp-specification.

OK... I've given this more thought, and there is a chance that users may want to create stamps dynamically, which will make it unsafe to throw, assuming that it's a conscious "design time" decision every time a stamp gets created -- in that case, it's possible that a nefarious actor could intentionally throw a wrench in the app to force constant throw + restart.

So let's assume throw is off the table. Let's do this:

• With the exception of the prototype, instance properties have lowest priority
• Shallows override deeps
• Descriptors override everything
• Optional warn on collision at stamp creation time (off by default)

You can turn on warn on collision with a configuration stamp:

const config = compose({
  configuration: {
    warnOnStampCollisions: true
  });

const newsFeedComponent = compose(twitterHashtagFollower, eventEmitter, hashTagEmitter, statusUpdatesComponent, config);

Thoughts?

from stamp-specification.

I like this. 👍 to optional warnings.

from stamp-specification.

Looks good to me too. Let's cement this in the specs.

from stamp-specification.

Note you have 2 flows to spec - what happens during compose() and what happens during instance creation.

The "optional-warning/throw" feature seems like implementation detail that may or may not be implemented. It does not look like thing that needs to be in spec.

Important thing is to write down what happens during instance creation - in what order state is installed so behaviour is predictible and without bad surprises.

from stamp-specification.

@markokr Thanks. I forgot about that. We all know the sequence, and it's kind of obvious to us.

Instantiating objects:
0) Create object, assign prototype (methods).

1. Assign Properties (do not overwrite anything existing)
2. Merge Deep Properties (do not overwrite anything existing)
3. Call intializers.
   Done.

The compose() operations are already listed here: https://github.com/stampit-org/stamp-specification#composing-descriptors

If we missed something in the specs please point. Or better submit a PR. We'd love to add you to the collaborators of this project. :)

from stamp-specification.

What you just described is the buggy flow - if those overwrite checks are missing, bad things happen (eg. stampit.js).

But also note that instance creation is the fastpath for this code, any extra checks should be avoided there, unless really no other way. We are talking about fancier way to do "new FooObj()", remember?

And those checks are not necessary there - just swap 1) and 2) - merge deep props first into empty object, then you can install shallow props without any worries. All overwrite checks can be done in compose().

from stamp-specification.

Not sure where are you pointing at in the specs. I am very glad you also have a good understanding of the things. :) Please, submit a PR. We'd love someone to improve the specs.
Thanks!

PS:

Premature optimization is the root of all evil.

from stamp-specification.

I was pointing at your comment.

Premature performace optimization is evil indeed, esp. if it increases complexity, but not premature complexity-optimization. I was simply pointing out that showing path to simpler code results also in faster performance, in addition to decreasing chance for bugs.

The spec should be the place for complexity-optimizations, as the interactions possible during various state operations and instance initializations can get rather messy.

from stamp-specification.

To clarify the current proposal:

1. There are no checks or warnings at instantiation time. There are optional checks at compose time. Optional checks SHOULD be respected by implementations if they exist.
2. Passed instances are the defaults. Everything else gets applied on top of the existing instance.
3. Deeps get merged in first (shallows override deeps).
4. Descriptors override everything.

from stamp-specification.

Sounds good to me.

from stamp-specification.

Merged.

from stamp-specification.