-
Notifications
You must be signed in to change notification settings - Fork 50
Additional strong-mode fixes (down to 0 after this PR) #13
Additional strong-mode fixes (down to 0 after this PR) #13
Conversation
@@ -37,7 +37,7 @@ abstract class Result<T> { | |||
/// The result of the transformation is a sink that only forwards [Result] | |||
/// values and no error events. | |||
static const StreamSinkTransformer<Object, Result> captureSinkTransformer = | |||
const StreamSinkTransformer.fromStreamTransformer( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jmesserly Why isn't this inferred? Same with the other changes in this file and stream_sink_transformer.dart
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think because const CaptureStreamTransformer()
is a StreamTransformer<dynamic, Result>
and not a StreamTransformer<Object, Result>
I think we could fix this instead with const CaptureStreamTransformer<Object>()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without seeing the error msg, it's hard to say for sure. I don't think dynamic vs Object would itself be problematic because those are treated almost identically for the subtype relation. You could try const CaptureStreamTransformer<Object>()
and see if that fixes the problem.
Perhaps also worth trying without the const
. const
goes down pretty different code paths in Analyzer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(the fact that the type annotation is required looks like some sort of bug to me, but of what sort or whether we have it filed already, I'm not certain)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes there was a warning. I'm happy to file a tracking bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you could, thanks! ideally w/ enough info to reproduce & things you tried to make the warning go away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this went away, somehow. I have no idea why. Reverted changes.
@@ -139,16 +139,16 @@ void main() { | |||
test("transforms error events", () { | |||
var transformer = new StreamSinkTransformer.fromHandlers( | |||
handleError: (i, stackTrace, sink) { | |||
sink.addError(i * 2, stackTrace); | |||
sink.addError((i as num) * 2, stackTrace); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it work to do instead:
handleError: (num i, stackTrace, sink) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not valid in checked-mode without covariant override
…async#13) * Additional strong-mode fixes * Revert infererred types
/cc @natebosch