Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port locking infrastructure to writable streams #319

Closed
domenic opened this issue Apr 7, 2015 · 6 comments
Closed

Port locking infrastructure to writable streams #319

domenic opened this issue Apr 7, 2015 · 6 comments

Comments

@domenic
Copy link
Member

domenic commented Apr 7, 2015

Writable streams need a similar way of being locked for exclusive writing, as readable streams do with their readers. This is notably crucial for off-main-thread piping (#97).

One problem that immediately presents itself, looking at the current list of writable stream properties and methods (closed/ready/state/abort()/close()/write()), is that pretty much all of them belong on the writer. Does that mean writable streams themselves become empty objects with getWriter() methods!?

I guess we could add a few pass-throughs (like ReadableStream.prototype.cancel acquiring, then releasing, a reader). Do we just add pass-through for everything? Well, probably not for ready/state/closed at least, as that causes problems. But if all we have is abort()/close()/write() passthroughs, doesn't that encourage the bad practice of ignoring backpressure signals? Maybe just abort() and close()?

@tyoshino
Copy link
Member

tyoshino commented Apr 8, 2015

become empty objects with getWriter() methods!?

Yeah, pipeTo etc. all belongs to the readable side.

What do you mean by pass-through? Keeping a method also on the WritableStream? As you just said, I also think we should basically limit them to WritableStreamWriter as much as possible.

just abort() and write()?

Maybe you meant abort() and close().

@domenic
Copy link
Member Author

domenic commented Apr 8, 2015

What do you mean by pass-through? Keeping a method also on the WritableStream?

I mean, like we do for ReadableStream.prototype.cancel, have versions of them on the WritableStream which throw if the WritableStream is locked.

Maybe you meant abort() and close().

Yes, editing OP.

@tyoshino
Copy link
Member

tyoshino commented Apr 9, 2015

OK. I slightly prefer not putting them on WritableStream. The same about .cancel(). Basically.

But given that we have .cancel() on ReadableStream, maybe it's better to make them consistent. I.e. have abort() or both abort() and close().

@domenic
Copy link
Member Author

domenic commented Aug 20, 2015

Plan:

class WritableStream {
  constructor(underlyingSink = {}, { size, highWaterMark = 0 } = {})

  get locked()

  getWriter()
  abort(reason)
  close()
}

class WritableStreamWriter {
  constructor(stream)

  get closed()
  get ready() // see #318

  abort(reason)
  close()
  releaseLock()
  write(chunk)
}

No need for WritableStreamController. Can eliminate error parameter to underlying sink start().

@tyoshino
Copy link
Member

PR created: #462

@tyoshino
Copy link
Member

tyoshino commented Aug 4, 2016

Fixed.

#462 has been succeeded by #488.

Only pass-through for abort() has been added.

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

No branches or pull requests

2 participants