Migrate to null safety #443
Conversation
|
BAAM, thanks a lot @miDeb . About |
Looks like you're right, in fact after simply removing the dependency rotating still seems to still work. |
| ChewieController chewieController; | ||
| AnimationController playPauseIconAnimationController; | ||
| late VideoPlayerController controller; | ||
| ChewieController get chewieController => _chewieController!; |
There was a problem hiding this comment.
Why can we not use a late ChewieController here?
There was a problem hiding this comment.
The problem is the logic in didChangeDependencies. This is the only place where _chewieController is used directly, and this is required because it tries to compare the old ontroller with the new controller. That would not work with late the very first time a controller is set, because there is no old controller and a LateInitializationError would be triggered.
The way I wrote this the rest of the widget (especially build methods) don't have to do a null check every time, but didChangeDependecies can have access to a nullable _chewieController.
Hope that makes sense :)
There was a problem hiding this comment.
Nice solution then.
Last idea I had was using initState, but then we cannot get the correct context for inheritedWidget.
Could you add a comment for the getter: Something like // We know that _chewieController is set in didChangeDependencies
There was a problem hiding this comment.
Sure, that will make it clearer for readers. Done.
|
I have now migrated the example as well; note that I removed |
|
Very awesome !!! |
| @@ -37,17 +37,19 @@ class _ChewieDemoState extends State<ChewieDemo> { | |||
| void dispose() { | |||
| _videoPlayerController1.dispose(); | |||
| _videoPlayerController2.dispose(); | |||
| _chewieController.dispose(); | |||
| _chewieController?.dispose(); | |||
There was a problem hiding this comment.
This change is not strictly related to the migration, but it makes the test pass.
| await _videoPlayerController1.initialize(); | ||
| _videoPlayerController2 = VideoPlayerController.network( | ||
| 'https://assets.mixkit.co/videos/preview/mixkit-a-girl-blowing-a-bubble-gum-at-an-amusement-park-1226-large.mp4'); | ||
| await _videoPlayerController2.initialize(); | ||
| await Future.wait([ | ||
| _videoPlayerController1.initialize(), | ||
| _videoPlayerController2.initialize() | ||
| ]); |
There was a problem hiding this comment.
The changes here are also to make the test pass again.
|
Awesome PR! Hope this will make people happy :D |
|
Published as 1.0.0 |
Hi! I noticed that all dependencies of this package are ready for null safety, so I figured I could quickly migrate chewie to null safety.
I am aware that this is a fairly big change, but I hope that most of it is straightforward.
You'll notice that I have removed some null checks, simply because the checked value could never be null and the check would therefore always return
true. For example,_latestValue(in Material-/CupertinoControls) is set right away in methods called frominitState, so with null safety it can be marked aslateand other methods don't have to do unnecessary null checks.I tested various features in the example, and everything seems to work. I didn't migrate the example itself yet though, because
auto_orientationdoes not yet support null safety (I opened a pull request to resolve this).There seems to be one test in the example, which fails for me before and after this patch. Do you rely on manual testing only?