cc @skabet.
cc @lrhn.

We've taken the first step in this process, by initiating a move of _StreamSinkImpl to dart:async as StreamSinkAdapter.

CL: https://codereview.chromium.org/196423021

This provides most of the logic for the IOSink, and should be sufficient in some cases.

Now, the remaining question is what should happen to IOSink. Some possibilities:

1. Stay in dart:io.
2. Move to dart:typed_data
3. Move to dart:convert
4. Copied/renamed, to some package, perhaps package:typed_data.

What do you think?

cc @floitschG.

We've taken the first step in this process, by initiating a move of _StreamSinkImpl to dart:async as StreamSinkAdapter.

👍

Now, the remaining question is what should happen to IOSink. Some possibilities:

1. Stay in dart:io.
2. Move to dart:typed_data
3. Move to dart:convert
4. Copied/renamed, to some package, perhaps package:typed_data.

What do you think?

I'm okay with any of the last three of those. I'd prefer #­2 or #­3, since I think it's useful for users to be able to use the same API in dart:io that they do elsewhere. Anders was reticent about adding a dependency from dart:typed_data onto dart:async, so #­3 may be the best option overall.

Agree with moving it to dart:convert

Disagree with moving to convert.

If you want a StringSink on top of a StreamSink in dart:convert, I would prefer a wrapper, not an extension. The class in dart:convert should not expose the StreamSink interface.

This is really about writing binary data interleaved with text, not about converting. I think it fits well in dart:io for that, or in a helper package.

This is really about writing binary data interleaved with text, not about converting.

I don't think this is quite right. It's about being able to write binary data or text; I don't think interleaving is the issue so much as being able to choose either mode with minimal fuss. When someone is writing to an HTTP stream or a file, they need the ability to write binary data, but the majority of the time in practice they want to write text. This interface makes that easy.

I'd also contend that writing text to something that expects binary data is fundamentally about converting.

I think it fits well in dart:io for that

I don't understand this claim. Practically everything dart:io does with IOSink also needs to be done in the browser where dart:io is unavailable. The browser needs to make HTTP requests, it needs to write to files, it needs to communicate with sockets. If IOSink is a good interface for that on the server (which I think it is), it should also be made available on the client.

or in a helper package.

In a helper package it's unavailable to dart:io.

If that is an either-or, the StreamSink<List<int>> handles the binary data
fine, and a StringSink wrapper would handle text fine.

If the use case is to send binary data and text to the same receiver, it
should be something that uses convert to convert the text to bytes, and
that also supports the binary data.
I don't see why that belongs in dart:convert, because it is just a case of
using dart:convert.

You can get that behavior with a simple wrapper around
StreamSink<List<int>>. Either you wrap it, for text, or you don't, for
binary data. It seems an odd amalgam to have a single class that does both.

Actually, I notice that ChunkedConversionSink is such a general type that
it should probably have had a more generic name, and StreamConsumer should
implement it. That would make us already have that wrapper by combining
dart:convert classes.

Absolutely. So you use a converter. I just don't think the converter and
the binary data output should be the same thing. And if they are, it
doesn't belong in dart:convert.

I guess my problem is that I don't think it's a good basic interface. Sure,
one object that does everything is convenient, but it's not very good
encapsulation or separation of concerns.

It is easy to make. We can make a mixin that does it: it converts every
call to a write* method into a call to add. Then we can mix it onto both a
StreamSink<List<int>> and a ByteConversionSink, but we don't need to do it
unless it is needed. Then IO could have its IOSink which is
"StreamSinkAdapter with StringWriterAddMixin", and anyone else can make
their own too, and I avoid having the combination in any single core
library :).

Does it need an interface? It's just a class that is both a StreamSink and
a StringSink. You can use it as either, but do you need a name for the type
that is both?

It sounds like you don't like IOSink in general. If so, why is it even in dart:io?

In general, I agree that it's good to have separate classes for separate jobs. However, there are cases where convenience is an overriding concern. You don't want users to always have to write var stringStdout = new ByteConversionSink(stdout), so you expose a class that does that automatically while still supporting writing bytes when necessary. The same is true for HTTP requests and files: users usually want to write strings but occasionally want to write bytes, so it makes sense to support both cases.

The problem is that we can't then expose the same interface (and leverage the users' familiarity with it) anywhere else. A package that wants to support both dart:io and dart:html has to reinvent the wheel/

I don't particularly like IOSink as a general interface. Mainly because it isn't very general.
It makes sense for things like stdout where you are likely to use both string output and binary output. It is a more specialized cases, and I don't object to having more specialized interfaces for that.
Personally, I would still have split it up. One example could be a stdout that is a StringSink with a getter "binary" that returns a StreamSink<List<int>>. Then you could do: stdout.write("my bytes: ");stdout.binary.add(someBytes);.

But that's done now. For the current problem, first notice that there are two problems being solved by IOSink: Adapting a StreamConsumer to be a StreamSink, and extending a StreamSink to also be a StringSink. Now that we are trying to generalize the interface and implementation, we have put the adapter in dart:async.

If we also want an interface that is both a StreamSink and a StringSink, with the StringSink delegating to the StreamSink through a converter, then I would still put the interface in dart:async. The class is still primarily a StreamSink, it just has extra helper methods for adding text data as bytes, which internally uses dart:convert functionality. The StringStreamSink interface will just be:
  class StringStreamSink implements StreamSink<List<int>>, StringSink, ChunkedConversionSink<List<int>>;
or it could even implement ByteCoversionSink.

Then dart:io can have an implementation of this interface.

We can also add a mixin class that allows adding the conversion features on top of any StreamSink<List<int>>.

I don't think we should have an actual public implementation on top of StreamSinkAdapter (except for what dart:io wants to have), because that is combining the two independent concerns (adapter, string extension) which should not be done at a generic level, but only in an actual specialized implementation class that combines the features it needs for a particular task.

So my suggestion is:

• put the StringStreamSink interface in dart:async
• maybe put a StringSinkAddMixin in dart:async too, with write methods that delegate to an abstract add method (then it can be used to mix-in on both a StreamSink and a ByteConversionSink)
• Let dart:io make its own:
    class IOSink = StreamSinkAdapter with StringStreamAddMixin implements StringStreamSink;

That sounds reasonable, but I'm still worried about how difficult it is to create a StringSink that emits a Stream<List<int>>. What would the code snippet be to make a StringSink given a StreamController<List<int>>?

You can do:
  var c = new StreamController<List<int>>();
  var s = UTF8.encoder.startChunkedEncoding(c).asStringSink();
(after I changed startChunkedEncoding to accept a Sink<T> instead of only a ChunkedConversionSink<T>, and StreamControler is-an EventSink is-a Sink).

That s will only be a StringSink, not both a StringSink and a StreamSink with flush.

Is there any way we can consolidate that? It seems very difficult for users to write. Maybe something like:

    new ChunkedStringController(encoding: UTF8)

where ChunkedStringController is both a StringSink and a StreamController<List<int>>.

My goal here is to make it easy for a user to expose a byte stream that can be written to with strings.

Agreeing with #­12

Also, having a settable encoding on IOSink feels a bit broken.

Removed Type-Defect label.
Added Type-Enhancement label.

If this is still necessary, I think the convert package is the right place for it rather than the core libraries.