useEffect's disposer should be called before update

[jeiea]

What I did and description

I don't know whether it is a bug but I'll explain it first.

I thought dispose function (printing unmount in the following code) should be called first after first mount. But it seems not.

Is it intended?

git clone -b effect_hook https://github.com/jeiea/flutter_example.git

import 'package:flutter/material.dart';
import 'package:flutter_hooks/flutter_hooks.dart';

void main() {
  runApp(MyApp());
}

class MyApp extends HookWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: MyHomePage(),
    );
  }
}

class MyHomePage extends HookWidget {
  MyHomePage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    final tapped = useState<bool?>(null);
    final logs = useState([]);

    useEffect(() {
      if (tapped.value == true) {
        // logs.value = [...logs.value, 'useEffect tap'];
        print('mount 1: ${logs.value}');
      } else if (tapped.value == false) {
        // logs.value = [...logs.value, 'useEffect tap 2'];
        print('mount 2: ${logs.value}');
      }
      return () {
        // logs.value = [...logs.value, 'useEffect unmount'];
        print('unmount: ${logs.value}');
      };
    }, [tapped.value != true]);

    return Scaffold(
      body: SafeArea(
        child: Padding(
          padding: const EdgeInsets.all(16.0),
          child: Column(
            mainAxisAlignment: MainAxisAlignment.center,
            children: <Widget>[
              SizedBox(
                width: double.infinity,
                child: ElevatedButton(
                  onPressed: () {
                    print('tap');
                    tapped.value = tapped.value != true;
                  },
                  child: Text('Tap me'),
                ),
              ),
              Expanded(
                child: ListView.builder(
                  itemCount: logs.value.length,
                  itemBuilder: ((context, index) => Text(logs.value[index])),
                ),
              ),
            ],
          ),
        ),
      ),
    );
  }
}

Expected behavior

flutter: mount 2: []
flutter: tap
flutter: unmount: []
flutter: mount 1: []
flutter: tap
flutter: unmount: []
flutter: mount 2: []

Actual behavior

flutter: mount 2: []
flutter: tap
flutter: mount 1: []
flutter: unmount: []
flutter: tap
flutter: mount 2: []
flutter: unmount: []

[rrousselGit]

Not a bug, but that could be a reasonable change.

[jeiea]

AFAIK react follow this. It's surprising to me that I didn't know this until now.

[jeiea]

It may be a breaking change but if I make a PR then it can be merged?

[rrousselGit]

Honestly I'm not sure. If that's how React behaves. there's some value in keeping it that way. And nobody complained so far either.

If this gets a bunch of 👍, sure. But for now I'd prefer to wait for the confirmation that other people want this too.

[kenNg1]

@jeiea Can you reproduce this code in React and share so that we can be certain it's inconsistent?

[jeiea]

Is the stackoverflow answer enough?

[cbreezier]

I was trying to write something like this:

    final debounceProgressUpdate = useRef<Timer?>(null);
    useEffect(() {
      final progress = scrollProgress.value;

      debounceProgressUpdate.value = Timer(Duration(seconds: 1), updateProgress);

      return () {
        debounceProgressUpdate.value?.cancel();
      };
    }, [scrollProgress.value]);

However, it doesn't work due to the order of cleanup and new effect.

I found the useDebounced hook that may allow me to write this another way, but it's still very surprising that flutter hooks runs the cleanup afterwards.