[WIP] Change many SHOULDs to MUSTs to ensure clear behavior

[kelunik]

For interfaces like that it's really important to have clear semantics.
There's e.g. no reason to allow "close" not being emitted after "error"
or "end".

[clue]

I like the idea of making our interfaces stricter, but I'm not sure if this is actually reasonable and how we should enforce your suggested changes.

For example, take the following quite common gist:

$stream->on('data', function ($chunk) use ($stream) {
    if (strpos($chunk, 'a') !== false) {
        $stream->close();
    }
});

$stream->on('data', function () { echo '.'; }
$stream->on('close', function () { echo 'closed'; }

The above gist is designed to close the stream once a certain character is matched (such as parsing a higher level protocol etc.). One would probably expect this to return something like .....closed. However, because the close() method is called as part of the data event, the close event will actually interrupt the final data event and this may actually return something like ....closed.. In particular, note how this also depends on the order the event listeners have been added and applies to all other events as well.

I'd love to hear some more thoughts about this. What do you think about this?

[kelunik]

I think the ordering here should be clearly defined. Either always call the 'close' event handler immediately or call the 'close' event handler after all remaining 'data' listeners have been called.

Another solution would be not calling the second 'data' listener at all after a 'close', but not sure whether that's a good idea, probably not.

[clue]

@kelunik While I think the current documentation already defines "clear behaviour", I think you're making a valid point here. However, I've just marked this PR as incomplete because it lacks the required code changes.

The current documentation was introduced mostly via #72 and #73 and (for the most part) documents how this component has always worked since its inception.

I'd like to emphasize that I think your suggest changes make perfect sense from a standards body perspective – however, they require a major BC break and it's currently unclear how much would be gained from a user perspective by introducing this.

We currently try to focus our efforts on stabilizing things and would like to tag a stable release of this component in the upcoming weeks. Addressing this issue in time would require a substantial effort from our side – with very little practical gain for our users on the other side.

This basically means that if you or anybody else feels like picking this up and addressing the above issues in time for the next stable release, then I'm all for getting this in! As such, PRs are highly welcome! Thank you!

[kelunik]

Closing this for now, as I don't have time currently.