Skip to content

Conversation

@liyuqian
Copy link
Contributor

@liyuqian liyuqian commented Sep 3, 2019

Reverts #38861

Reason for revert: Stream is considered a bad API: flutter/engine#11041 (comment)

@fluttergithubbot fluttergithubbot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Sep 3, 2019
@devoncarew
Copy link
Contributor

I'm not clear what the issues with the Stream API are. Do we have a proposal for what would replace the API? The issues with the previous API where that clients of the API had to manage ownership of a field, and would likely mostly do that unsuccessfully.

@liyuqian
Copy link
Contributor Author

liyuqian commented Sep 3, 2019

@devoncarew : yes, I'm thinking about storing the callbacks as a list like addPostFrameCallback. @Hixie : does that sound good to you?

@hpoul
Copy link
Contributor

hpoul commented Sep 3, 2019

I'm also curious about the problems with streams? it doesn't seem too long ago that BLoC + stream was the go-to solution.. is there some major difference between framework use of streams, vs. framework users? I tend to follow the lead of the framework to have some uniform solution vs. having everything doing things differently (callbacks vs Listenable/ChangeNotifier vs stream).

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@dnfield
Copy link
Contributor

dnfield commented Sep 4, 2019

@Hixie
Copy link
Contributor

Hixie commented Sep 4, 2019

Streams are fine in general, they're just not the best solution to this problem.

WidgetsBinding.instance.window.onReportTimings = oldCallback;
_firstFrameCompleter.complete();
});
};
Copy link
Contributor

Choose a reason for hiding this comment

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

this all probably belongs in the SchedulerBinding, fwiw.

@liyuqian liyuqian merged commit 7c0dfd5 into master Sep 5, 2019
@tvolkert tvolkert deleted the revert-38861-use_frame_timings branch December 3, 2019 18:49
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants