Sitelet https://github.com/nodejs/node/issues/55
Skip to content

Proposal: Move isaacs/readable-stream to iojs and make it the authoratative source #55

Description

@rvagg

#53 and all of the other obscure streams issues that keep coming up keep on giving me headaches. Even though I "maintain" readable-stream as an extraction of joyent/node streams I still don't have a complete handle on the inner workings of the streams code and I imagine this is the same for many core contributors. I'm willing to bet that even now, nobody understands the code better than @isaacs because it's grown so complex.

As the most obvious source of JavaScript complexity in core, I propose that we move readable-stream to iojs and make it the place to file streams issues and where the code in core is extraced from rather than the other way around. It can also have its own core team, separate but potentially overlapping with iojs TC, that cares about and understands how they work and maybe the tight integration with core can be loosened (and perhaps broken completely) over time. As a separate project we can also pursue better testing (including browserify) of the code.

Activity

  1. feross commented on Dec 4, 2014

    @feross
    Contributor

    +1 I'm hugely in favor of this.

  2. rvagg commented on Dec 4, 2014

    @rvagg
    MemberAuthor

    @feross be warned, you'd be on my list of people to pull in to a maintainer team if this goes ahead

  3. jonathanong commented on Dec 4, 2014

    @jonathanong
    Contributor

    +1

  4. isaacs commented on Dec 4, 2014

    @isaacs
    Contributor

    +1. @rvagg do you need me to do the switch?

  5. fengmk2 commented on Dec 4, 2014

    @fengmk2
    Contributor

    +1

  6. rvagg commented on Dec 4, 2014

    @rvagg
    MemberAuthor

    @isaacs at this stage I'm more looking for agreement from TC to reverse the flow of code so that all the work is done in readable-stream rather than core, but there's no harm in moving readable-stream over to the org in the meantime.

  7. isaacs commented on Dec 4, 2014

    @isaacs
    Contributor

    Oh, I see, so the plan would be to have the module be the "upstream" and then iojs consumes it?

    If so, that makes a lot of sense, and moving it to the org also seems sensible. I think this should be the exception rather than the norm, however. Stuff like util-debuglog should probably still remain as-is for now.

  8. ahdinosaur commented on Dec 4, 2014

    @ahdinosaur

    👍

  9. tellnes commented on Dec 4, 2014

    @tellnes
    Contributor

    +1

  10. indutny commented on Dec 4, 2014

    @indutny
    Member

    Finally, +1

  11. missinglink commented on Dec 4, 2014

    @missinglink
    Contributor

    +1 happy to help

  12. bnoordhuis commented on Dec 4, 2014

    @bnoordhuis
    Member

    +1 as long as I don't have to triage bugs. :-)

  13. chrisdickinson commented on Dec 4, 2014

    @chrisdickinson
    Contributor

    I'm a little worried about making readable-stream (the package) the authoritative source for streams in Node/iojs -- though at the moment I'm having difficulty putting the "why" down into words. I'll revisit this later today and see if I can't explain better, in the meantime I'm "-0" for that change (it could just turn out that I'm being silly about this, though.) I'm totally +1 putting the package under the iojs org, though!

  14. chrisdickinson commented on Dec 5, 2014

    @chrisdickinson
    Contributor

    Okay, here goes:

    Issues with and PRs against streams will be opened against this repository. The latter can be solved with a big ole' disclaimer comment at the top of the vendored file, but the former splits issues between "those who know where readable-stream is" and "those who don't." Additionally, It could have the side effect of making stream issues less visible to streams maintainers, who would have to look in two places to track current problems (and, due to the gravity-like effect of interest on attention, will likely end up following the readable-stream tracker way more closely than the iojs tracker).

    maybe the tight integration with core can be loosened (and perhaps broken completely) over time

    As one of node's core primitives, I'm not sure loosening or breaking streams the integration of streams with core is feasible; even if it is feasible, I am not sure that it would be a good thing. Streams are one of the few things that packages must agree on. Anecdotally, js-git is a good example of what a world without standard stream primitives would look like -- where the stream primitive is a local agreement between cooperating packages.

    In order to have my streams1-based git packages work with js-git, I rewrote them into min-streams and added translation modules. The min-streams only depended on other min-streams), and exposed streams1 to other consumers on the side. This took a lot of time, and while it made the module usable from min-streams or streams1, it still wouldn't necessarily work with pull-streams, or RxJS; and it only worked in a single package because I had a verbal agreement with the consumers (all two of us) of these packages -- I knew that the min-stream version would be available at <package>/min.js. Were this not the case, there would likely be a streams-package-A and a min-package-A, which is the sort of lock-in effect that streams effectively combatted.

    One of the really lovely things about streams as a primitive is that modules are casually pluggable. There's not so much specification as to get in the way of the problems being solved, or so little specification that more time is spent providing shim modules at edges between ecosystems, but just enough agreement to foster cooperation and enable the community to do some truly cool things. My experience integrating streams1 with min-streams for js-git led me to believe that local, ecosystem-based agreements for the stream primitive are far less effective than global agreement. Min-streams aren't bad -- they're really neat, and for the constraints js-git works within they make total sense -- but both streams1 and min-streams lost a lot of effectiveness by not being the globally agreed-upon standard interface.

    (And, given all that, it follows that I believe that the readable-stream module and this repo would have to be upgraded in lockstep for most changes to readable-stream, which would create more work, and thus more potential for human error than currently exists.)

    I'm willing to bet that even now, nobody understands the code better than @isaacs because it's grown so complex.

    Oof, I definitely agree with this. I have a tiny stack of hand drawn state machine diagrams with a lot of frustrated scribbles in the margins that I consult regularly. We need to decrease the bus factor, and forming an impromptu team of people that can field issues with streams is a great idea.
    That said, I don't think moving streams out of core is a precondition to forming that team.

    I'm a little concerned about having the option to "solely" work in streams. Having streams in core provides JS developers who are aspiring contributors a nice "ladder" to climb up into different subsystems (and C++). The design of streams, on the other hand, benefits from informed use in core -- and I think there are still gains to be made there (uniting resource/handle-backed streams under a common subclass, or otherwise addressing pain points with the existing integrations).

    Also of note: stream documentation -- due to the complexity of streams, they hit a lot of pain points with the way documentation is currently done in Node (is it a guide? is it reference material? who is the audience?) Moving the symptom out of Node proper makes it less likely that the underlying problem will get addressed satisfactorily -- when docs are revisited, they should be revisited with the pain of streams docs fresh in mind, and with solutions in hand.

    Sorry for the unstructured stream of concerns. To recap, in numbered list form:

    1. Moving streams into a repo doesn't move the issues out of this repo.
      1. But it does make stream maintainers have to look in two repos for pertinent issues.
      2. It adds a step in PR'ing streams changes.
      3. It divides the audience into folks who know where to look and folks who don't.
    2. I do not think that decoupling streams from core will be to either core or stream's benefit.
      1. Streams work best as global agreements between all packages.
      2. Changes to a readable-stream repo will likely have to happen in lock-step with changes to this repo.
        1. This adds manual steps, which increases the potential for human error.
    3. Streams are complex enough to deserve their own repo
      1. I think that having to work with streams in-situ improves the design of streams.
      2. Having streams in core gives new contributors who are familiar with JS a good place to start.
      3. Removing the onus of documenting streams from this repo means that updates to the docs subsystem are less likely to keep the pain points of streams in mind.
  15. 7 remaining items

  16. DamonOehlman commented on Dec 10, 2014

    @DamonOehlman

    +1 for bringing it in, +1 for @feross as a potential maintainer

  17. rvagg commented on Dec 10, 2014

    @rvagg
    MemberAuthor

    TC today agreed to make this happen, at least as an experiment for now, @rvagg and @chrisdickinson to take responsibility for the mechanics of making this happen.

    Need to consider the flow of:

    • code
    • docs
    • issues

    between the two repos

    TC meeting notes available in #144

  18. Raynos commented on Dec 17, 2014

    @Raynos
    Contributor

    Note that if readable-stream is in iojs we need to most important artifact.

    The reason readable-stream exists is that you can require('readable-stream').

    I wouldn't mind require('iojs/stream') but depending on all of iojs might be a large dependency.

    Where the code lives is not important, the fact that readable-stream gets published to npm is important

  19. Fishrock123 commented on Jan 22, 2015

    @Fishrock123
    Contributor

    readable-stream has long been moved to https://github.com/iojs/readable-stream

    They also have (almost?) finalized their working group: nodejs/readable-stream#97

    Deferring to there, we can cycle back to discussions on whether or not to make it authoritative in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions