Skip to content
This repository has been archived by the owner on Oct 17, 2024. It is now read-only.

Deprecate DelegatingStream.typed and make it delegate to Stream.cast #54

Merged
merged 11 commits into from
Mar 3, 2018

Conversation

leafpetersen
Copy link
Contributor

I removed stream_test.dart since all of the functionality is really part of the SDK implementation of Stream.cast now, but if you'd prefer me to keep it around let me know.

@nex3
Copy link
Contributor

nex3 commented Mar 2, 2018

This is great! Do you mind doing the same for the other Delegating*.typed() methods?

@leafpetersen
Copy link
Contributor Author

We didn't add cast/retype to Future and StreamSubscription, so we can't do quite the same thing.

For the Future case, using

  static Future<T> typed<T>(Future future) =>
      future is Future<T> ? future : future.then((x) => x as T);

does the right thing typing wise, I think, but it doesn't handle the unwrapping/rewrapping if you cast it again.

@@ -1,5 +1,5 @@
name: async
version: 2.0.5
version: 2.0.6
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this available in -dev.23?

Do we need to bump the SDK constraint?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, these APIs were available in dev.23, and all tests for this package pass with dev.23.

@leafpetersen
Copy link
Contributor Author

Just pushed a patch removing internal uses of the now deprecated .typed method, and also ran the formatter.

@nex3
Copy link
Contributor

nex3 commented Mar 2, 2018

We didn't add cast/retype to Future and StreamSubscription, so we can't do quite the same thing.

Why not? It may be useful to be able to cast the type argument of pretty much any generic class.

If we can't get rid of all the typed() methods, I'd rather leave DelegatingStream.typed() un-deprecated so that people can access these uniformly.

@leafpetersen
Copy link
Contributor Author

Why not? It may be useful to be able to cast the type argument of pretty much any generic class.

I don't know, I wasn't part of the design discussions for these. @lrhn maybe knows?

If we can't get rid of all the typed() methods, I'd rather leave DelegatingStream.typed() un-deprecated so that people can access these uniformly.

Ok, will do.

@leafpetersen
Copy link
Contributor Author

Done, PTAL.

[skip ci]
@nex3 nex3 merged commit c6809e7 into master Mar 3, 2018
@nex3 nex3 deleted the corelib_2_2_2 branch March 3, 2018 01:15
@nex3
Copy link
Contributor

nex3 commented Mar 3, 2018

Thanks Leaf!

@leafpetersen
Copy link
Contributor Author

Thanks, and thanks for publishing!

@lrhn
Copy link
Contributor

lrhn commented Mar 6, 2018

We didn't add cast/retype to Future and StreamSubscription, so we can't do quite the same thing.

Why not? It may be useful to be able to cast the type argument of pretty much any generic class.

We had to stop somewhere, and there didn't seem to be any cases where these were needed.

The Future isn't a collection, it's just a single value, so any time you check the type of the value, you already have the value in hand and it's as easy to create a new future as it is to wrap the existing one.

The StreamSubscription just isn't used much as a value that is passed around. You pass the stream around, and then use the subscription in one place.

Also, the await and await for syntax makes the classes less relevant. You can do:

int x = await numFuture;
await for (int y in numStream) ....

without needing to cast the future or collection first, even if you can't do numFuture.then((int x) ...) or numStream.listen((int y) ...), because they both do assignments at the value level, not the collection/future level.

So, basically, we didn't think it was worth the API complexity to add the feature. It was always mainly an Iterable and Stream (collection, really) feature, not a general feature intended for any generic class.

mosuem pushed a commit to dart-lang/core that referenced this pull request Oct 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

6 participants