Composer redesign

[molily]

This is my attempt to understand and clean up the composer a bit. It also (roughly) an implementation of the ideas from #643 (comment).

I’ll explain the details in the next couple of days. This PR is more for discussion because there are a lot of breaking and unrelated changes here.

[paulmillr]

NOT nice if we don't want to be like Backbone and break compat each semver-valid release.

Since we are on 1.x now, we should not break it with 1.x.

[akre54]

I disagree. Making changes is part of moving forward. We should try to limit the number of changes between major releases to be sure, but non-major functionality or feature changes shouldnt require a major version to publish.

[paulmillr]

So, can you tell briefly what's this about? (I don't remember; i've looked through discussion)

As I understand, instead of @reuse there will be @retrieve, @reuse and @share.

Why do we need three APIs? cc @YAndrew91

[molily]

The current code in the PR is a breaking change, yes. I’m not against maintaining backwards compatibility, it’s just not what I had in mind with this quick sketch. This is more a working demo of a possible future API. Well, I don’t know yet if it works in real applications, that’s what I want to find out with this.

The idea is to clean up the existing API and make it clearer, for users and internally. One reuse with five plus one signatures is confusing. Not all cases are tested properly and only two are documented. The internal naming is also confusing IMHO (Composer, Composer#(_)compose, Composition, Composition#compose, Composition#item).

There are a lot of subtleties and edge cases that I stumbled upon while writing the specs and the implementation. My approach was Delete your Code and to question everything we have, and to write more specs to the illuminate the edge cases.

@reuse is still the main controller method and still supports the signatures name, Constructor, options, name, {create: Function, check: Function} and name, CustomComposition, options, but not the (currently tested, but undocumented) functional form. I’m unsure about putting everything into one function that has multiple different signatures and return values. The deprecation error you have commented on was mainly to postpone this question for now.

My questions are:

• Where to create a composition, providing all necessary information (like constructor, options; {create, check}). Then of course share it with the next controller.
• When to create a composition by Constructor, options, when by {create, check}, when by CustomComposition, options. (These options are useful, but is there a way to merge then without losing power?)
• Where to just access a composition created by the same or a previous controller without providing the creation information and without sharing it again necessarily. (Idea behind this: The creation code should not be duplicated. Maybe it’s the best only to work with a separate CustomComposition?)
• Where to share an existing object with the next controller. No check if a previous controller has shared it, therefore no receiving. (Legit or even worse then the reuse signatures?)

I’d like to figure out if and when these operations are necessary, how the use cases look and how best practices could look.

As I said, I will explain more details in the next days.

[molily]

Notes for me (WIP):

• Different return values (object vs composition vs promise). Return values were mostly untested (due to the Pub/Sub past). Promise missing here, was untested.
• For this reason, composition.item pointed to the composition itself when using a CustomComposition. Seemed hacky to me. Now there is a general check for composition.object. Is this really better? What’s most clear?
• For the promise, there was an exception. Not using composition.item. Why? If the composition holds an object, the promise would the be right object.
• My current idea: Only the simple form should set composition.object.
• While developing, I moved the simple create code to Composition#create where it belongs IMO. But the signature was inconsistent, so I reverted this. The closure in Composer#makeCreateObject is not nice. What’s the best way? Call could be composition.create(options, constructor).
• reuse 'name', Constructor with a different constructor does not recreate the composition (does not in master either). Should it? Same goes same name but different {create, check} IIRC.
• Previously, a new composition instances was always created – even if current.check returns true. Necessary? Harmful? Was this about being able to process the options in CustomComposition#initialize?

[akre54]

Anything I can help with here?