Using new parameter 'interval' (v. 0.2.0) causes exception

[budo76]

When the widget is removed from the widget tree it still tries to animate, causing the following exception:

AnimationController methods should not be used after calling dispose.
#0      _AssertionError._doThrowNew (dart:core-patch/errors_patch.dart:46:39)
#1      _AssertionError._throwNew (dart:core-patch/errors_patch.dart:36:5)
#2      AnimationController.forward (package:flutter/src/animation/animation_controller.dart:456:7)
#3      _ShimmerAnimatorState.initState.<anonymous closure>.<anonymous closure> (package:shimmer_animation/src/shimmer_animator.dart:40:60)
 #4      new Future.delayed.<anonymous closure> (dart:async/future.dart:326:39)

In brief the internally used AnimationController has already been disposed (correctly) when the internal Future (used to delay the shimmer) tries to animate forward.

To reproduce the problem just create a basic app with a button to perform a Navigator.push to push a route, with a Page that use the Shimmer with the "interval" param, and then just press "back" or programmatically add a button to perform the Navigator.pop in order to remove the Shimmer widget from the widget-tree. Wait the 'interval' delay to see the exception.

[budo76]

Perhaps at line 40 of shimmer_animator.dart you just need to check the mounted property of the State. Something like:

[...] // line 37
..addListener(() async {
        if (controller.isCompleted)
          Future.delayed(widget.interval, () {
            if (mounted) controller.forward(from: 0);
          });
[...]

... in theory after the widget is removed from the tree (disposed) the state is unmonted so it will never try to animate after that.

EDIT: I tested this change (directly modifying the sources from the flutter directory) and it works, so it would be useful to integrate this patch in the project for next release...

[maddyb99]

Thank you for raising this issue. I would integrate this ASAP or would you like to open a Pull request, since I see that you have already tried and tested this?

[budo76]

Since it's just an "if" statement it would be simpler for you integrate this, so I don't have to clone the project just for this... you know, I just changed that line "on the fly" on the flutter downloaded directory and tested "on the field" (not automated test or anything), not really the correct workflow.. just to see if it was "just that"... Thanks. Il lun 12 ott 2020, 16:47 Madhav Baveja <notifications@github.com> ha scritto:

…

Thank you for raising this issue. I would integrate this ASAP or would you like to open a Pull request, since I see that you have already tried and tested this? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALAIBCYGBXI7HIYG2H3Z7LSKMJH3ANCNFSM4SMPARIQ> .

[maddyb99]

I totally agree, will do it, its just that I won't be able to push it out now due to some reasons, would try to do it in a couple of days!

Thanks again for sharing this 🙌