qlevar_router icon indicating copy to clipboard operation
qlevar_router copied to clipboard

Build method is getting fired after onExitFunc middleware gets executed by Page

Open tejHackerDEV opened this issue 1 year ago • 1 comments

Hi @SchabanBo this issue is same as #75, as it is closed by you opening a new issue. This happens with GetX same as mentioned in the tagged issue. I am using binding concept of GetX using the middleware concept that we have in Qlevar i.e., I am registering the dependencies in "onEnterFunc" & removing the dependencies in "onExitFunc" (same as the way you suggested in #71). Everything works fine but the problem happens when user navigates between screens when keyboard is opened ie., let suppose say user is interacting with a form & keyboard is open or some text-input fields has focus & now lets say user clicks on a button to navigate to some other page by replacing the currentPage then the currentPage "onExitFunc" will get trigger then all the dependencies will get clear but after that build method of the currentPage is getting called again as keyboard is opened or some inputs has focus. With this what happens is that as in build method we are using controller fields which are removed from memory eventually giving flutter default red screen error.

So my suggestion will be same like "onExitFunc", its better to have one more middleware callback called as "onExitedFunc" or "onDisposeFunc" in which we do these kinds of dependency removal or memory cleaning to avoid this kind of issues.

For now in order to suppress the error in "onExitFunc" I am using some delay with Future.delayed (like a second) & removing the dependencies after the delay

Below is the reproducible code, just add getX dependency in the pubspec.yaml to make code buildable

import 'package:flutter/material.dart';
import 'package:get/get.dart';
import 'package:qlevar_router/qlevar_router.dart';

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

class MyApp extends StatelessWidget {
  final routes = [
    QRoute(
      name: 'sign-in',
      path: '/sign-in',
      middleware: [
        QMiddlewareBuilder(
          onEnterFunc: () async {
            Get.put(SignInController());
          },
          onExitFunc: () async {
            Get.delete<SignInController>();
          },
        ),
      ],
      builder: () => const SignInPage(),
    ),
    QRoute(
      name: 'sign-up',
      path: '/sign-up',
      middleware: [
        QMiddlewareBuilder(
          onEnterFunc: () async {
            Get.put(SignUpController());
          },
          onExitFunc: () async {
            Get.delete<SignUpController>();
          },
        ),
      ],
      builder: () => const SignUpPage(),
    ),
  ];

  MyApp({super.key});

  @override
  Widget build(BuildContext context) => MaterialApp.router(
        routeInformationParser: const QRouteInformationParser(),
        routerDelegate: QRouterDelegate(routes, initPath: '/sign-in'),
        theme: ThemeData.dark(),
      );
}

class SignInPage extends GetView<SignInController> {
  const SignInPage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) => Scaffold(
        appBar: AppBar(
          title: Text('Sign In'),
          centerTitle: true,
        ),
        body: Column(
          children: [
            TextField(
              controller: controller.textEditingController,
            ),
            ElevatedButton(
              onPressed: () => QR.navigator.replaceLastName('sign-up'),
              child: const Text('Sign-Up'),
            ),
          ],
        ),
      );
}

class SignInController extends GetxController {
  final TextEditingController textEditingController = TextEditingController();

  @override
  void onClose() {
    textEditingController.dispose();
    super.onClose();
  }
}

class SignUpPage extends GetView<SignUpController> {
  const SignUpPage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) => Scaffold(
        appBar: AppBar(
          title: Text('Sign Up'),
          centerTitle: true,
        ),
        body: Column(
          children: [
            TextField(
              controller: controller.textEditingController,
            ),
            ElevatedButton(
              onPressed: () => QR.navigator.replaceLastName('sign-in'),
              child: const Text('Sign-In'),
            ),
          ],
        ),
      );
}

class SignUpController extends GetxController {
  final TextEditingController textEditingController = TextEditingController();

  @override
  void onClose() {
    textEditingController.dispose();
    super.onClose();
  }
}

tejHackerDEV avatar Oct 11 '22 18:10 tejHackerDEV

Hi @tejHackerDEV, I had the same problem in a project, and I solved it in the same way as you. I added Future.delayed. This would be a good idea, to run a function after ensuring that the page is completely removed. I will add this in the next release. Thanks for suggestions.

SchabanBo avatar Oct 12 '22 09:10 SchabanBo

Added in 1.7.0

SchabanBo avatar Oct 27 '22 07:10 SchabanBo

Hi @SchabanBo, thanks for the fix but this issue still persists. Below are the things that I found while testing

  1. Newly added onExitedFunc is not getting called if used with replaceLastName. This is because of not adding middleware.scheduleOnExited(); // schedule on exited at this line
  2. I am using this onExitedFunc to delete the dependencies(GetX Controllers required by the page), but even after executing the onExitedFunc the build method is getting called which results in flutter red screen error, because GetXController that belongs to page that has gone out of scope has been deleted via onExitedFunc but as build is triggered after that function the deleted controller is being used.

tejHackerDEV avatar Oct 27 '22 16:10 tejHackerDEV

@SchabanBo, upon further testing I found below things

The second issue that I referenced above is happening because in the example code we have replaceLastName but as per the first issue I referenced while replaceLastName onExitedFunc is not getting (but it supposed to be called in that case too as page gone out of the scope). So dependencies are not getting cleared, so there is no red screen. while navigating between pages, you can replace "replaceLastName" with "replaceAllWithName" then you will get red screen (because onExitedFunc will be called in replaceAllWithName)

So it should happen in the following ways.

  1. Even with replaceLastName or replaceAllWithName onExitedFunc should be triggered.
  2. Once onExitedFunc triggered build method of that particular page should never be called

tejHackerDEV avatar Oct 27 '22 17:10 tejHackerDEV

  1. Newly added onExitedFunc is not getting called if used with replaceLastName. This is because of not adding middleware.scheduleOnExited(); // schedule on exited at this line

That is right, it should be here. I will add this.

For the second issue (If I understood this right) the problem is, that the build method of the previous page will be called after OnExitedFunc is called?!!

In this smaple the build method will not be called after the store page

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

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

class MyApp extends StatelessWidget {
  static List<String> tabs = [
    "Home Page",
    "Store Page",
    "Settings Page",
  ];

  static int indexOf(String name) => tabs.indexWhere((e) => e == name);
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    final routes = [
      QRoute.withChild(
          path: '/home',
          builderChild: (c) => HomePage(c),
          children: [
            QRoute(
              name: tabs[0],
              path: '/',
              builder: () => const Tab('Home'),
            ),
            QRoute(
              name: tabs[1],
              path: '/store',
              middleware: [
                QMiddlewareBuilder(
                  onEnterFunc: () async => print('Enter'),
                  onExitFunc: () async => print('Exit'),
                  onExitedFunc: () => print('Exited'),
                )
              ],
              builder: () => const Tab('Store'),
            ),
            QRoute(
              name: tabs[2],
              path: '/settings',
              builder: () => const Tab('Settings'),
            ),
          ]),
    ];
    return MaterialApp.router(
      routeInformationParser: const QRouteInformationParser(),
      routerDelegate: QRouterDelegate(routes, initPath: '/home'),
      theme: ThemeData.dark(useMaterial3: true),
    );
  }
}

class HomePage extends StatefulWidget {
  final QRouter router;
  const HomePage(this.router, {Key? key}) : super(key: key);
  @override
  State<HomePage> createState() => _HomePageState();
}

class _HomePageState extends RouterState<HomePage> {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: const Text('My App')),
      body: widget.router,
      bottomNavigationBar: BottomNavigationBar(
        items: const [
          BottomNavigationBarItem(icon: Icon(Icons.home), label: 'home'),
          BottomNavigationBarItem(icon: Icon(Icons.store), label: 'store'),
          BottomNavigationBarItem(icon: Icon(Icons.settings), label: 'Settings')
        ],
        currentIndex: MyApp.indexOf(widget.router.routeName),
        onTap: (v) => QR.toName(
          MyApp.tabs[v],
          // Add this line if you want to keep all of your tab alive(which will save the state for it)
          //pageAlreadyExistAction: PageAlreadyExistAction.BringToTop,
        ),
      ),
    );
  }

  @override
  QRouter get router => widget.router;
}

class Tab extends StatefulWidget {
  final String name;
  const Tab(this.name, {Key? key}) : super(key: key);

  @override
  State<Tab> createState() => _TabState();
}

class _TabState extends State<Tab> {
  var counter = 0;
  @override
  Widget build(BuildContext context) {
    print('Building ${widget.name}');
    return Column(
      mainAxisAlignment: MainAxisAlignment.center,
      children: [
        Text(widget.name, style: const TextStyle(fontSize: 20)),
        Text(counter.toString(), style: const TextStyle(fontSize: 20)),
        IconButton(
          onPressed: () {
            setState(() {
              counter++;
            });
          },
          icon: const Icon(Icons.add),
        ),
      ],
    );
  }
}

Can you provide me an example for this?

SchabanBo avatar Oct 28 '22 11:10 SchabanBo

Use the above provided example only, it will throw issue in that itself

tejHackerDEV avatar Oct 28 '22 12:10 tejHackerDEV

That is really weird. But here the build method will not be called again (what we expect). Check this. Oct-28-2022 15-10-13 The ----------- Build sign up is shown only one time in the logs. That means the package didn't call the build method again.

SchabanBo avatar Oct 28 '22 13:10 SchabanBo

So after a lot of debugging the issue seemed with focus node of the text field. I just made the text field to unfocus before leaving the page, and it worked.

See the code here
import 'package:flutter/material.dart';
import 'package:get/get.dart';
import 'package:qlevar_router/qlevar_router.dart';

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

class MyApp extends StatelessWidget {
  final routes = [
    QRoute(
      name: 'sign-in',
      path: '/sign-in',
      middleware: [
        QMiddlewareBuilder(
          onEnterFunc: () async {
            Get.put(SignInController());
          },
          onExitedFunc: () async {
            Get.delete<SignInController>();
          },
        ),
      ],
      builder: () => const SignInPage(),
    ),
    QRoute(
      name: 'sign-up',
      path: '/sign-up',
      middleware: [
        QMiddlewareBuilder(
          onEnterFunc: () async {
            Get.put(SignUpController());
          },
          onExitedFunc: () async {
            print('object');
            Get.delete<SignUpController>();
            print('object');
          },
        ),
      ],
      builder: () => const SignUpPage(),
    ),
  ];

  MyApp({super.key});

  @override
  Widget build(BuildContext context) => MaterialApp.router(
        routeInformationParser: const QRouteInformationParser(),
        routerDelegate: QRouterDelegate(routes, initPath: '/sign-in'),
        theme: ThemeData.dark(),
      );
}

class SignInPage extends GetView<SignInController> {
  const SignInPage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) => Scaffold(
        appBar: AppBar(
          title: const Text('Sign In'),
          centerTitle: true,
        ),
        body: Column(
          children: [
            TextField(
              focusNode: controller.focus,
              controller: controller.textEditingController,
            ),
            ElevatedButton(
              onPressed: () => QR.navigator.replaceAllWithName('sign-up'),
              child: const Text('Sign-Up'),
            ),
          ],
        ),
      );
}

class SignInController extends GetxController {
  final TextEditingController textEditingController = TextEditingController();
  final focus = FocusNode();

  @override
  void onClose() {
    focus.unfocus();
    focus.dispose();
    textEditingController.dispose();
    super.onClose();
  }
}

class SignUpPage extends GetView<SignUpController> {
  const SignUpPage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    print('----------- Build sign up');
    return Scaffold(
      appBar: AppBar(
        title: const Text('Sign Up'),
        centerTitle: true,
      ),
      body: Column(
        children: [
          TextField(
            focusNode: controller.focus,
            controller: controller.textEditingController,
          ),
          ElevatedButton(
            onPressed: () => QR.navigator.replaceAllWithName('sign-in'),
            child: const Text('Sign-In'),
          ),
        ],
      ),
    );
  }
}

class SignUpController extends GetxController {
  final TextEditingController textEditingController = TextEditingController();
  final focus = FocusNode();

  @override
  void onClose() {
    focus.unfocus();
    focus.dispose();
    textEditingController.dispose();
    super.onClose();
  }
}

SchabanBo avatar Oct 28 '22 14:10 SchabanBo

@SchabanBo, please check the PR #91 which fixes the above issue, also with some enhancements to the package

tejHackerDEV avatar Oct 30 '22 15:10 tejHackerDEV

Fixed in #96, will be available in next release

tejHackerDEV avatar Nov 06 '22 09:11 tejHackerDEV