rrousselGit / flutter_hooks

React hooks for Flutter. Hooks are a new kind of object that manages a Widget life-cycles. They are used to increase code sharing between widgets and as a complete replacement for StatefulWidget.
MIT License
3.14k stars 179 forks source link

state.value = newIndex is not causing a re-build of the widget even though newIndex is different #434

Open nateshmbhat opened 4 months ago

nateshmbhat commented 4 months ago

Describe the bug Calling activeIndex.value = newIndex is not causing a re-build of the widget.

To Reproduce Full Reproducible code :

class PracticeDetailScreen extends HookConsumerWidget {
  final YogaPracticeId practiceId;
  const PracticeDetailScreen({super.key, required this.practiceId});

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final practiceData = getYogaPracticeData(practiceId);
    final activeIndex = useState<int>(0);

    if (practiceData == null) {
      return const SAppScaffold(
        body: Center(
          child: Text('Practice not found'),
        ),
      );
    }
    return SAppScaffold(
      appBarOptions: SAppBarOptions(title: practiceData.title),
      body: SafeArea(
        child: Column(
          children: [
            TabBarRow(onTabChanged: (newIndex) {
              activeIndex.value = newIndex;
            }),
            activeIndex.value == 0
                ? const IntroTabView()
                : const MeditateTabView()
          ],
        ),
      ),
    );
  }
}

class TabBarRow extends HookConsumerWidget {
  const TabBarRow({super.key, required this.onTabChanged});
  final Function(int) onTabChanged;

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final activeIndex = useState<int>(0);
    useEffect(() {
      onTabChanged(activeIndex.value);
    }, [activeIndex.value]);

    return Container(
      margin: const EdgeInsets.symmetric(horizontal: 20),
      height: 50,
      padding: const EdgeInsets.symmetric(horizontal: 5),
      decoration: ShapeDecoration(
        color: Colors.white,
        shape: RoundedRectangleBorder(
          side: const BorderSide(width: 1, color: Color(0xFFE8EAEB)),
          borderRadius: BorderRadius.circular(6),
        ),
      ),
      child: Row(
        mainAxisSize: MainAxisSize.min,
        mainAxisAlignment: MainAxisAlignment.center,
        crossAxisAlignment: CrossAxisAlignment.center,
        children: [
          Expanded(
            child: TabBarButton(
              onPressed: () {
                activeIndex.value = 0;
              },
              isActive: activeIndex.value == 0,
              title: 'Intro',
            ),
          ),
          Expanded(
            child: TabBarButton(
                onPressed: () {
                  activeIndex.value = 1;
                },
                isActive: activeIndex.value == 1,
                title: 'Meditate'),
          ),
        ],
      ),
    );
  }
}

class TabBarButton extends StatelessWidget {
  const TabBarButton(
      {super.key,
      required this.onPressed,
      required this.isActive,
      this.title = 'Intro'});

  final bool isActive;
  final VoidCallback onPressed;
  final String title;

  @override
  Widget build(BuildContext context) {
    return SPressable(
      onPressed: () {
        onPressed();
      },
      child: Container(
        height: 40,
        padding: const EdgeInsets.symmetric(horizontal: 14, vertical: 10),
        decoration: ShapeDecoration(
          color: isActive ? const Color(0xFF1FA1AA) : null,
          shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(5)),
        ),
        child: Column(
          mainAxisSize: MainAxisSize.min,
          mainAxisAlignment: MainAxisAlignment.center,
          crossAxisAlignment: CrossAxisAlignment.center,
          children: [
            Text(
              title,
              style: TextStyle(
                color: isActive ? Colors.white : SColors.darkGrey,
                fontSize: 14,
                fontFamily: 'Open Sans',
                fontWeight: FontWeight.w600,
                height: 0,
              ),
            ),
          ],
        ),
      ),
    );
  }
}

class IntroTabView extends HookConsumerWidget {
  const IntroTabView({super.key});
  @override
  Widget build(BuildContext context, WidgetRef ref) {
    return Container(
      height: 100,
      width: 50,
      color: Colors.blue,
      child: const Text('intro'),
    );
  }
}

class MeditateTabView extends HookConsumerWidget {
  const MeditateTabView({super.key});
  @override
  Widget build(BuildContext context, WidgetRef ref) {
    return Container(child: const Text('meditate'));
  }
}

Package versions : hooks_riverpod: ^2.5.1 flutter_hooks: ^0.20.5 riverpod_annotation: ^2.3.5

Expected behavior Clicking on the "Intro" button or "Meditate" button should show the corresponding IntroTabView and MeditateTabView .

nateshmbhat commented 4 months ago

Can u please check on this? This looks like a critical issue in useState...

rrousselGit commented 4 months ago

I'm in holiday. But try:

    useEffect(() {
      Future(() => onTabChanged(activeIndex.value));
    }, [activeIndex.value]);

I'm pretty sure you have an exception somewhere in your code because of this useEffect.

It's violating some widget rules, which would lead to Flutter not updating the UI.

nateshmbhat commented 4 months ago

Can someone pls tell me which widget rules is violated? This is the full code I pasted which can be run in any project.

Pablets commented 4 months ago

I hardcoded missing data on your example because it's not reproducible as it is.

When I make it run and press on the tab buttons this error appears:

Exception has occurred.
FlutterError (setState() or markNeedsBuild() called during build.
This PracticeDetailScreen widget cannot be marked as needing to build because the framework is already in the process of building widgets. A widget can be marked as needing to be built during the build phase only if one of its ancestors is currently building. This exception is allowed because the framework builds parent widgets before children, which means a dirty descendant will always be built. Otherwise, the framework might not visit this widget during this build phase.
The widget on which setState() or markNeedsBuild() was called was:
  PracticeDetailScreen
The widget which was currently being built when the offending call was made was:
  TabBarRow)

So, when PracticeDetailScreen is still building there is another child widget triggering a rebuild in PracticeDetailScreen again.

This code "just works" meaning that I'm not 100% sure if these hooks are meant to be used like that, sorry I come from react and I din't find any documentation about this but the reasoning is this:

The state must be in one parent widget (PracticeDetailScreen) and be passed down to your child widgets as props. Instead you are making a new state inside TabBarRow and somehow (I didn't understand that part of the implementation) trying to sinchronize it with the parent state via a useEffect and a funtion.

and the code is this:

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

class PracticeDetailScreen extends HookConsumerWidget {
  // I don't know what `YogaPracticeId` is so I leave it hardcoded.
  final int practiceId;
  // I put a default value, with 0 you trigger a empty response.
  const PracticeDetailScreen({super.key, this.practiceId = 1});

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    // The data it's not important in this case so we can leave it hardcoded
    final practiceData = practiceId != 0 ? null : 'some data';
    final activeIndex = useState<int>(0);

    if (practiceData == null) {
      return const Scaffold(
        body: Center(
          child: Text('Practice not found'),
        ),
      );
    }
    // Removed unncessesary dependencies
    return Scaffold(
      body: SafeArea(
        child: Column(
          children: [
            TabBarRow(activeIndex: activeIndex),
            activeIndex.value == 0
                ? const IntroTabView()
                : const MeditateTabView()
          ],
        ),
      ),
    );
  }
}

class TabBarRow extends StatelessWidget {
  // Here we receive the state `activeIndex` from the parent that is of type  ValueNotifier<int>
  final ValueNotifier<int> activeIndex;
  const TabBarRow({super.key, required this.activeIndex});

  @override
  Widget build(BuildContext context) {
    return Container(
      margin: const EdgeInsets.symmetric(horizontal: 20),
      height: 50,
      padding: const EdgeInsets.symmetric(horizontal: 5),
      decoration: ShapeDecoration(
        color: Colors.white,
        shape: RoundedRectangleBorder(
          side: const BorderSide(width: 1, color: Color(0xFFE8EAEB)),
          borderRadius: BorderRadius.circular(6),
        ),
      ),
      child: Row(
        mainAxisSize: MainAxisSize.min,
        mainAxisAlignment: MainAxisAlignment.center,
        crossAxisAlignment: CrossAxisAlignment.center,
        children: [
          Expanded(
            child: TabBarButton(
              onPressed: () {
                activeIndex.value = 0;
              },
              isActive: activeIndex.value == 0,
              title: 'Intro',
            ),
          ),
          Expanded(
            child: TabBarButton(
                onPressed: () {
                  activeIndex.value = 1;
                },
                isActive: activeIndex.value == 1,
                title: 'Meditate'),
          ),
        ],
      ),
    );
  }
}

class TabBarButton extends StatelessWidget {
  const TabBarButton(
      {super.key,
      required this.onPressed,
      required this.isActive,
      this.title = 'Intro'});

  final bool isActive;
  final VoidCallback onPressed;
  final String title;

  @override
  Widget build(BuildContext context) {
    return ElevatedButton(
      onPressed: () {
        onPressed();
      },
      child: Container(
        height: 40,
        padding: const EdgeInsets.symmetric(horizontal: 14, vertical: 10),
        decoration: ShapeDecoration(
          color: isActive ? const Color(0xFF1FA1AA) : null,
          shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(5)),
        ),
        child: Column(
          mainAxisSize: MainAxisSize.min,
          mainAxisAlignment: MainAxisAlignment.center,
          crossAxisAlignment: CrossAxisAlignment.center,
          children: [
            Text(
              title,
              style: TextStyle(
                color: isActive ? Colors.white : Colors.grey,
                fontSize: 14,
                fontFamily: 'Open Sans',
                fontWeight: FontWeight.w600,
                height: 0,
              ),
            ),
          ],
        ),
      ),
    );
  }
}

class IntroTabView extends HookConsumerWidget {
  const IntroTabView({super.key});
  @override
  Widget build(BuildContext context, WidgetRef ref) {
    return Container(
      height: 100,
      width: 50,
      color: Colors.blue,
      child: const Text('intro'),
    );
  }
}

class MeditateTabView extends HookConsumerWidget {
  const MeditateTabView({super.key});
  @override
  Widget build(BuildContext context, WidgetRef ref) {
    return const Text('meditate');
  }
}

I would investigate a bit more if I were you, maybe some experienced flutter dev can help, but in the meantime it works. If this approach have unknown pitfalls then I think the other option is to use a provider.