[RFC] Improve compatibility with legacy versions #53
Conversation
| use React\Stream\WritableStreamInterface; | ||
| use React\Stream\Stream; | ||
| use React\Stream\Util; | ||
|
|
||
| class SecureStream extends Stream implements DuplexStreamInterface | ||
| class SecureStream extends Stream |
There was a problem hiding this comment.
What about code that typehints against DuplexStreamInterface? Considering to be a BC tbh.
There was a problem hiding this comment.
This actually amends #26 which in turn amends #14.
The documentation always stated this returns a Stream instance, never an instance implementing DuplexStreamInterface.
FWIW: This repo currently targets react/stream:0.4.*, while this interface has only been added with v0.4.2, so even if we decide not to support legacy versions, this is still a bug that needs fixing. If you happen to have v0.4.2+, then Stream implements the DuplexStreamInterface.
As such, I do not consider this a BC break.
Even despite this, this PR should probably be part of the v0.5.0 milestone, which already includes a (minor) BC break anyway.
There was a problem hiding this comment.
Wasn't aware DuplexStreamInterface was added later in 0.4.x, so yeah it isn't a BC break imo :).
|
Not quite sure what it was but I get a failing unit test when merging this into master (even after resolving the merge conflict). |
I've just rebased this now that #52 is in and all tests are working just fine 👍 What error are you seeing? |
|
Rebase looks good. I must have messed up the merge on my end before |
Because why not… :-)
Now more seriously: This project is a low level lib that is used as a building block for quite a few higher level abstractions on top of it. As such, compatibility (even with significantly outdated versions) is a major concern to me.
Note that I'm not suggesting putting significant amount of work into this. The patch is already here and, personally, I see little harm in supporting this.
Also note that I'm not suggesting we need to keep support indefinitely. Should this ever turn out to be a burden in the future, e.g. because we actually require any new language features or some external lib, then I'm all for dropping support again.