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

StreamTransformer is difficult to use in Dart 2.0 #29878

Open
vsmenon opened this issue Jun 14, 2017 · 14 comments
Open

StreamTransformer is difficult to use in Dart 2.0 #29878

vsmenon opened this issue Jun 14, 2017 · 14 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. core-n enhancement-breaking-change An enhancement which is breaking. library-async type-enhancement A request for a change that isn't a bug

Comments

@vsmenon
Copy link
Member

vsmenon commented Jun 14, 2017

This has come up several times, but I don't see a bug for it.

Here's a sample use that highlights the problem:

import 'dart:async';

Stream<int> change(Stream<Iterable> stream) {
  return stream.transform(new StreamTransformer<Iterable, int>.fromHandlers(
      handleData: (Iterable iter, EventSink<int> sink) {
    sink.add(iter.length);
  }));
}

void main() {
  Stream<Iterable> stream = new Stream<List<String>>.fromIterable([
    ["hello"],
    ["world"]
  ]);
  var result = change(stream);
  result.listen(print);
}

The change method looks just fine from a typing perspective. However, if we invoke it with Stream<T> where T is a proper subtype of Iterable, it will trigger a confusing runtime error:

Type '_StreamHandlerTransformer<Iterable, int>' is not a subtype of type 'StreamTransformer<List<String>, int>'

Intuitively, StreamTransformer should be contravariant on it's first type parameter (Iterable here). But, Dart does not support that.

@vsmenon vsmenon added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. P1 A high priority bug; for example, a single project is unusable or has many test failures S1 high library-async labels Jun 14, 2017
@vsmenon
Copy link
Member Author

vsmenon commented Jun 14, 2017

A workaround is to modify the reified runtime type on the stream:

var result = change(stream.map<Iterable>((x) => x));

should do it.

@lrhn
Copy link
Member

lrhn commented Jun 15, 2017

True, it's annoying, but a direct consequence of Dart's covariant generic classes.

A _StreamHandlerTransformer<Iterable, int> is not a subtype of StreamTransformer<List<String>,int> because Iterable is not a subtype of List<String> - it's the other way around.

We don't have a way to specify, or detect, that a type parameter of a class occurs only contravariantly and should be treated as such.

@vsmenon
Copy link
Member Author

vsmenon commented Jun 26, 2017

@donny-dont

Here's another case we're hitting in package:http:

Simplified, we end up with something like this:

import 'dart:async';
import 'dart:convert';
import 'dart:typed_data';

final codec = new AsciiCodec();

Stream<List<int>> read() {
  // Reified as a Stream<Uint8List>
  var stream = new Stream.fromIterable(<Uint8List>[codec.encode("hello"), codec.encode("world")]);
  return stream;
}

void main() {
  // This throws: Type 'AsciiDecoder' is not a subtype of type 'StreamTransformer<Uint8List, String>'
  codec.decodeStream(read()).then(print);
}

@vsmenon
Copy link
Member Author

vsmenon commented Jun 26, 2017

@floitschG @kasperl

Is there something actionable here the language/sdk team can do to improve? We know what the problem is. How do we fix?

I've heard ideas including language changes or new APIs for this functionality, but not clear if we're proceeding on any front.

@matanlurey
Copy link
Contributor

+1 this is a major pain point. Honestly I'd love to see this just become a function definition:

typedef StreamTransformer = T Function<S, T>(Stream<S> input);

@floitschG
Copy link
Contributor

The correct solution is to introduce contravariant generics. We currently have no plans to add those.

Making StreamTransformer a typedef works in simple cases, but all important StreamTransformers are also Converters. Converters aren't simple functions that take one element and return another. They are a group of functions (convert, fuse, bind, ...). That screams for a "container" class to bundle them together (call it Converter) and you are back to square one.

@vsmenon
Copy link
Member Author

vsmenon commented Jun 26, 2017

@floitschG How do you want to proceed? I don't think the status quo is working.

@matanlurey
Copy link
Contributor

FWIW I have almost never used Converter, but I don't see why we couldn't support:

abstract class Converter<S, T> {
  const factory Converter.fromTransformer(StreamTransformer<S, T> transformer);
}

To handle this case.

@vsmenon
Copy link
Member Author

vsmenon commented Jun 26, 2017

E.g., a fairly minimal change:

https://codereview.chromium.org/2954383002/diff/1/sdk/lib/async/stream.dart

gets this last example running (and perhaps the rest?).

I'm not fond of writing dynamic (or Object if you prefer), but T is/was just incorrect.

@lrhn
Copy link
Member

lrhn commented Jun 27, 2017

@matanlurey The problem is not to make a StreamTransformer into a Converter, but that many transformers are already converters, because all converters are tranformers:

abstract class Converter<S, T> implements StreamTransformer<S, T> { 

That means that they will have the same co-/contra-variance problems as you are seeing with StreamTransformer, it's not just about that class.

Even if we make StreamTransformer into a function type so it's contravariant in the source type, that won't help Converter. you could extract a transformer from a converter by tearing off the bind method, but it wouldn't be a transformer unless we also add a call method that forwards to bind, and then you have function subtyping and class subtyping being at odds with each other. That's not going to be ... at all understandable.

class C<T> {
   void call(T x);
}
typedef F = void Function<T>(T);
main() {
   var i = new C<int>();
   var n  = new C<num>();
   print(n is C<int>);  // false
   print(n is F<int>);  // true
   print(n is C<Object>);  // true
   print(n is F<Object>);  // false
   C<Object> o = n;
   F<Object> fo = o;  // No warning, strong mode runtime error
}

So, we can perhaps fix the problem for transformer (if we make it a function type, not a class type), but that won't help Converter.

@matanlurey
Copy link
Contributor

matanlurey commented Jun 27, 2017 via email

@floitschG floitschG added core-n and removed core-m labels Aug 21, 2017
@vsmenon vsmenon changed the title StreamTransformer is difficult to use with strong mode / DDC StreamTransformer is difficult to use in Dart 2.0 Oct 2, 2017
@franklinyow
Copy link
Contributor

@munificent @lrhn
Please re-evaluate this and do one of the following:

  1. Add it to a milestone if it is still P1
  2. Change priority label and add milestone if no longer P1
  3. Close, if this issue is no longer needed

@vsmenon vsmenon removed the P1 A high priority bug; for example, a single project is unusable or has many test failures label Aug 11, 2020
@munificent
Copy link
Member

Definitely not a P1. I think this is worth revisiting once we ship the variance changes.

@lrhn
Copy link
Member

lrhn commented Aug 17, 2020

Agree. There isn't much we can do with the current API and feature-set.
Changing the API, say to it being a function type, would be breaking.
The variance feature would allow us to mark the input type of the transformer as contravariant, which would likely solve most of the issues, and there'd be a migration for that feature anyway.

@lrhn lrhn added enhancement-breaking-change An enhancement which is breaking. type-enhancement A request for a change that isn't a bug labels Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. core-n enhancement-breaking-change An enhancement which is breaking. library-async type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants