Dispose in reverse order about flutter_hooks HOT 29 CLOSED

We’ve just encountered the same problem. Can you please merge the fix?

from flutter_hooks.

Yes. I overestimated the consequences at that time.

The only situation where this could be surprising is, it may create some exceptions after a hot-reload that reorder hooks.
Considering hot-reload is dev only, I've estimated that it's not worth thinking about.

from flutter_hooks.

from flutter_hooks.

@brunodmn You can trye the useIsMounted hook. Check whether the state is mounted after the downloadFile function finishes.
https://pub.dev/documentation/flutter_hooks/latest/flutter_hooks/useIsMounted.html

from flutter_hooks.

Ah, indeed. I would swear I reversed it. I remember fixing the failing tests and everything...

Thanks, will do.

from flutter_hooks.

It's not that simple. While such change fixes a few things, it also breaks others.

Similarly, React doesn't dispose of them in reverse order. And clearly, they use hooks on a much larger scale

from flutter_hooks.

Could you merge it and just give the library a flag to define forward/reverse order.

from flutter_hooks.

@derolf You mentioned that this caused a lot of issues. Could you clarify what these are?

For now, I'm still not convinced if we should do something about it or not.

from flutter_hooks.

Generally, I think reverse order is natural since that is what you usually do in dispose/destructors: you dispose stuff in the reverse order of creation. That is especially important if there are dependencies among the things you create.

Concrete: I have a bunch of hooks to create scrollcontrollers, texteditingcontrollers etc. and then in a large fraction I am using useListenable to connect to them. During destruction, the controllers are killed first and then ListenableHook tries to remove itself which crashes the app.

from flutter_hooks.

The issue is, reverse order cause problems too because hooks have a "keys" that allow them to be disposed of on command.

So for example, we can have:

final controller = useMemoized(() => MyController(someKey), [someKey]);
useMyController(controller);

In that situation, we're stuck. Because changing someKey would dispose of the previous controller, but useMyController would still remove its listener after the disposal.

So having hooks disposed only from top to bottom allows some form of consistency.

from flutter_hooks.

Similarly, React doesn't dispose hooks in reverse order of creation, and their hooks are used on a much larger scale.

A potential solution may be to offer 3 kind of hooks instead of two (or at least 2 + a flag):

• a hook to create/dispose an object
• another one to listen to the object
• a third one that does both

Although this may not solve more advanced use-cases with animations and tweens and stuff.

from flutter_hooks.

In that case either order will be wrong since when someKey changes and the old controller gets killed, useMyController will crash once it attempts to unsubscribe from the old (now dead) controller.

Probably we need a solution that allows to "scope" hooks inside other hooks.

from flutter_hooks.

a third one that does both

That's also what I am thinking of. It would be nice if all Listenables in Flutter would implement a Disposable interface...

from flutter_hooks.

The issue being, React doesn't have this problem, because they never use Listenable equivalents.

I tried to speak a bit with Dan Abramov on that topic some months ago, but got no concrete solution out of it.

from flutter_hooks.

What do you propose to fix the following:

    final queryTextController = useTextEditingController();

    useEffect(() {
      final cbOnChanged = () { // do something usefull };
      queryTextController.addListener(cbOnChanged);
      return () => queryTextController.removeListener(cbOnChanged);
    });

This throws:

The following assertion was thrown while disposing _EffectHookState:
A TextEditingController was used after being disposed.

Should I just drop the removeListener? :D

from flutter_hooks.

I created our own variant of _ListenableHook:

  @override
  void dispose() {
    try {
      hook.listenable.removeListener(_listener);
    } catch (_) {
      // see https://github.com/rrousselGit/flutter_hooks/issues/81
    }
  }

from flutter_hooks.

Interesting fix!

I'm not too good with GC, would it potentially create memory leaks (And work with streams)?

from flutter_hooks.

@derolf i am running into the same issue. Are you still using the custom hook, or do you guys have something new?

from flutter_hooks.

As a comeback to that:

The issue is, reverse order cause problems too because hooks have a "keys" that allow them to be disposed of on command.

So for example, we can have:

final controller = useMemoized(() => MyController(someKey), [someKey]);
useMyController(controller);

In that situation, we're stuck. Because changing someKey would dispose of the previous controller, but useMyController would still remove its listener after the disposal.

So having hooks disposed only from top to bottom allows some form of consistency.

Maybe one possibility is to delay the disposal of hooks in that situation to the end of the frame.

By doing so, we would have the list of all the hooks that need to be disposed of. So we could do it in reverse order.

from flutter_hooks.

After all this time, I've finally decided to reverse the dispose order.
Sorry for the delay.

from flutter_hooks.

Sounds good, but why finally?

from flutter_hooks.

It's not that simple. While such change fixes a few things, it also breaks others.

Similarly, React doesn't dispose of them in reverse order. And clearly, they use hooks on a much larger scale

@rrousselGit I recall there were some limitations with an original draft you created. Is that solved?
Thanks for the update! 😄

from flutter_hooks.

Hi, today I updated from 0.9.0 to 0.14.1 and I received A ScrollController was used after being disposed after any hot reload.

Is there a way to fix this?

I use this code:

final scrollController = useMemoized(() => ScrollController());

useEffect(() {
  scrollController.addListener(onScroll);
  return () => scrollController.removeListener(onScroll);
}, [key]);

useEffect(() {
  return () => scrollController.dispose();
}, []);

from flutter_hooks.

Move your first useEffect after the second

from flutter_hooks.

@andrea689 you are calling the .removeListener after your .dispose
try this

final scrollController = useMemoized(() => ScrollController());

useEffect(() {
  return () => scrollController.dispose();
}, []);

useEffect(() {
  scrollController.addListener(onScroll);
  return () => scrollController.removeListener(onScroll);
}, [key]);

from flutter_hooks.

Thank you to all, it is working! Sorry but I'm new on hooks.

from flutter_hooks.

I am trying to rebuild my tabController depending on some feature flags being on or off.

final featureFlags = useState(database.getFeatureFlags());
database.featureFlagsBox.watch(key: "feature_flags").listen((event) {
      featureFlags.value = event.value;
    });
final allPages = useMemoized(() {
      return _buildPages(featureFlags.value);
    }, [featureFlags.value]);
final _ticker = useSingleTickerProvider();
final _tabController = useTabController(initialLength: allPages.length, vsync: _ticker, keys: [featureFlags.value]);

Doing that results in and error:

The following assertion was thrown building CollectionsPage(dirty, dependencies: [_EffectiveTickerMode, _LocalizationsScope-[GlobalKey#13ef1], _InheritedTheme], useState<FeatureFlags>: Instance of 'FeatureFlags', useMemoized<List<PageViewDescription>>: [Instance of 'PageViewDescription'], useSingleTickerProvider):
A TabController was used after being disposed.

Once you have called dispose() on a TabController, it can no longer be used.
The relevant error-causing widget was: 
  CollectionsPage .../lib/components/screens/home/HomeScreen.dart:44:67
When the exception was thrown, this was the stack: 
#0      ChangeNotifier._debugAssertNotDisposed.<anonymous closure> (package:flutter/src/foundation/change_notifier.dart:117:9)
#1      ChangeNotifier._debugAssertNotDisposed (package:flutter/src/foundation/change_notifier.dart:123:6)
#2      ChangeNotifier.dispose (package:flutter/src/foundation/change_notifier.dart:212:12)
#3      TabController.dispose (package:flutter/src/material/tab_controller.dart:299:11)
#4      _TabControllerHookState.dispose (package:flutter_hooks/src/tab_controller.dart:57:33)

from flutter_hooks.

Hello, I am having a similar problem with useState to show a progress dialog hud.

#this is my code init

@override
  Widget build(BuildContext context) {
    final showDialog = useState(false);

#this is my state change

showDialog.value = true;
await CoreMethods.downloadFile();           
 showDialog.value = false;

it works fine, unless I leave the screen while loading, in this case the state is disposed but code keeps running and try to set it anyway.

Would be nice if I could check whether showDialog is disposed before update the value (I wouldn't need to change value if its disposed already).

Is there some workaround or fix?

Thanks

from flutter_hooks.

@brunodmn You can trye the useIsMounted hook. Check whether the state is mounted after the downloadFile function finishes.
https://pub.dev/documentation/flutter_hooks/latest/flutter_hooks/useIsMounted.html

It worked this way, thank you! I'd tried with useIsMounted before, but not on build body.

bellow my updated snippets, for reference:

init...

 @override
  Widget build(BuildContext context) {
    final isMounted = useIsMounted();
    final _showDialog = useState(false);

state change

showDialog.value = true;
            await CoreMethods.downloadFile();
            if (isMounted()) {
              _showDialog.value = false;
            }
          }), 

from flutter_hooks.