jonataslaw / getx

Open screens/snackbars/dialogs/bottomSheets without context, manage states and inject dependencies easily with Get.
MIT License
10.33k stars 1.62k forks source link

Get.offNamed on routes with parameters doesn't close the route correctly #635

Closed tneotia closed 4 years ago

tneotia commented 4 years ago

Describe the bug I am using parameters in my route names to pass data between screens. Whenever I want to delete this screen from the tree, I would do Get.offNamed, but it doesn't delete the route and its controller. I get this in the logs:

[GETX] REPLACE ROUTE null
[GETX] NEW ROUTE /inbox?id=test

whereas with Get.back (which works properly), I get this:

[GETX] CLOSE TO ROUTE /inbox/message?id=78
[GETX] "MessageScreenController" onClose() called
[GETX] "MessageScreenController" deleted from memory

To Reproduce Steps to reproduce the behavior:

  1. Create app with 3 screens (named routes with parameters like Get.toNamed("/screenB?id=test"))
  2. From screen A, navigate to screen B and use Get.back() to go back
  3. From screen A, navigate to screen C and use Get.offNamed() to go back
  4. Observe the screen C's controller is not deleted while the screen B's controller is deleted

Expected behavior Screen C's controller should be deleted as well with Get.offNamed() and parameters.

Flutter Version: v1.21.0-pre

Getx Version: v3.10.2

Describe on which device you found the bug: ex: Galaxy S8 (Android 9)

fer-ri commented 4 years ago

Hi, please add a minimal reproduce code.

tneotia commented 4 years ago

@ghprod

Didn't get a chance to test but this should hopefully work. Basically press 'next route' on home screen, this will take you to named route with no params. Then tap back page and remove route, observe that in logs of [Getx] the route is removed and the controller is deleted.

Then go to next route and then click go to last page, and finally press the floating action button which will take you back to main screen. Again observe the logs and in this case it will remove a 'null' route and not delete the controller.

``` import 'package:flutter/material.dart'; import 'package:get/get.dart'; void main() { runApp(GetMaterialApp( // It is not mandatory to use named routes, but dynamic urls are interesting. initialRoute: '/home', defaultTransition: Transition.native, translations: MyTranslations(), locale: Locale('pt', 'BR'), getPages: [ //Simple GetPage GetPage(name: '/home', page: () => First()), // GetPage with custom transitions and bindings GetPage( name: '/second', page: () => Second(), customTransition: SizeTransitions(), binding: SampleBind(), ), // GetPage with default transitions GetPage( name: '/third', transition: Transition.cupertino, page: () => Third(), ), ], )); } class MyTranslations extends Translations { @override Map> get keys => { 'en': { 'title': 'Hello World %s', }, 'en_US': { 'title': 'Hello World from US', }, 'pt': { 'title': 'Olá de Portugal', }, 'pt_BR': { 'title': 'Olá do Brasil', }, }; } class Controller extends GetxController { int count = 0; void increment() { count++; // use update method to update all count variables update(); } } class First extends StatelessWidget { @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar( leading: IconButton( icon: Icon(Icons.add), onPressed: () { Get.snackbar("Hi", "I'm modern snackbar"); }, ), title: Text("title".trArgs(['John'])), ), body: Center( child: Column( mainAxisAlignment: MainAxisAlignment.center, children: [ GetBuilder( init: Controller(), // You can initialize your controller here the first time. Don't use init in your other GetBuilders of same controller builder: (_) => Text( 'clicks: ${_.count}', )), RaisedButton( child: Text('Next Route'), onPressed: () { Get.toNamed('/second'); }, ), RaisedButton( child: Text('Change locale to English'), onPressed: () { Get.updateLocale(Locale('en', 'UK')); }, ), ], ), ), floatingActionButton: FloatingActionButton( child: Icon(Icons.add), onPressed: () { Get.find().increment(); }), ); } } class Second extends GetView { @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar( title: Text('second Route'), ), body: Center( child: Column( mainAxisAlignment: MainAxisAlignment.center, children: [ GetX( // Using bindings you don't need of init: method // Using Getx you can take controller instance of "builder: (_)" builder: (_) { print("count1 rebuild"); return Text('${_.count1}'); }, ), GetX( builder: (_) { print("count2 rebuild"); return Text('${controller.count2}'); }, ), GetX(builder: (_) { print("sum rebuild"); return Text('${_.sum}'); }), GetX( builder: (_) => Text('Name: ${controller.user.value.name}'), ), GetX( builder: (_) => Text('Age: ${_.user.value.age}'), ), RaisedButton( child: Text("Go to last page"), onPressed: () { Get.toNamed('/third?arguments=test'); }, ), RaisedButton( child: Text("Back page and remove route"), onPressed: () { Get.offNamed('/home'); }, ), RaisedButton( child: Text("Increment"), onPressed: () { Get.find().increment(); }, ), RaisedButton( child: Text("Increment"), onPressed: () { Get.find().increment2(); }, ), RaisedButton( child: Text("Update name"), onPressed: () { Get.find().updateUser(); }, ), RaisedButton( child: Text("Dispose worker"), onPressed: () { Get.find().disposeWorker(); }, ), ], ), ), ); } } class Third extends GetView { @override Widget build(BuildContext context) { return Scaffold( floatingActionButton: FloatingActionButton(onPressed: () { Get.offNamed('/home'); }), appBar: AppBar( title: Text("Third ${Get.parameters['arguments']}"), ), body: Center( child: Obx(() => ListView.builder( itemCount: controller.list.length, itemBuilder: (context, index) { return Text("${controller.list[index]}"); }))), ); } } class SampleBind extends Bindings { @override void dependencies() { Get.lazyPut(() => ControllerX()); } } class User { User({this.name = 'Name', this.age = 0}); String name; int age; } class ControllerX extends GetxController { final count1 = 0.obs; final count2 = 0.obs; final list = [56].obs; final user = User().obs; updateUser() { user.update((value) { value.name = 'Jose'; value.age = 30; }); } /// Once the controller has entered memory, onInit will be called. /// It is preferable to use onInit instead of class constructors or initState method. /// Use onInit to trigger initial events like API searches, listeners registration /// or Workers registration. /// Workers are event handlers, they do not modify the final result, /// but it allows you to listen to an event and trigger customized actions. /// Here is an outline of how you can use them: /// made this if you need cancel you worker Worker _ever; @override onInit() { /// Called every time the variable $_ is changed _ever = ever(count1, (_) => print("$_ has been changed (ever)")); everAll([count1, count2], (_) => print("$_ has been changed (everAll)")); /// Called first time the variable $_ is changed once(count1, (_) => print("$_ was changed once (once)")); /// Anti DDos - Called every time the user stops typing for 1 second, for example. debounce(count1, (_) => print("debouce$_ (debounce)"), time: Duration(seconds: 1)); /// Ignore all changes within 1 second. interval(count1, (_) => print("interval $_ (interval)"), time: Duration(seconds: 1)); } int get sum => count1.value + count2.value; increment() => count1.value++; increment2() => count2.value++; disposeWorker() { _ever.dispose(); // or _ever(); } incrementList() => list.add(75); } class SizeTransitions extends CustomTransition { @override Widget buildTransition( BuildContext context, Curve curve, Alignment alignment, Animation animation, Animation secondaryAnimation, Widget child) { return Align( alignment: Alignment.center, child: SizeTransition( sizeFactor: CurvedAnimation( parent: animation, curve: curve, ), child: child, ), ); } } ```
fer-ri commented 4 years ago

Hi,

Next time please add minimal reproduce code that we can run on our local by just copy-pasting the code.

Regarding your issue, it's work fine with GetX 3.11.1

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

main() => runApp(App());

class App extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return GetMaterialApp(
      initialRoute: '/first',
      getPages: [
        GetPage(
          name: '/first',
          page: () => FirstScreen(),
          binding: BindingsBuilder(() {
            Get.lazyPut<FirstController>(() => FirstController());
          }),
        ),
        GetPage(
          name: '/second',
          page: () => SecondScreen(),
          binding: BindingsBuilder(() {
            Get.lazyPut<SecondController>(() => SecondController());
          }),
        ),
        GetPage(
          name: '/third',
          page: () => ThirdScreen(),
          binding: BindingsBuilder(() {
            Get.lazyPut<ThirdController>(() => ThirdController());
          }),
        ),
      ],
    );
  }
}

class FirstController extends GetxController {}

class SecondController extends GetxController {
  final title = 'Second'.obs;
}

class ThirdController extends GetxController {
  final title = 'Third'.obs;
}

class FirstScreen extends GetView<FirstController> {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: Column(
          mainAxisSize: MainAxisSize.min,
          children: [
            RaisedButton(
              onPressed: () {
                Get.toNamed('/second?id=x');
              },
              child: Text('Go to Second'),
            ),
            RaisedButton(
              onPressed: () {
                Get.toNamed('/third?id=x');
              },
              child: Text('Go to Third'),
            ),
          ],
        ),
      ),
    );
  }
}

class SecondScreen extends GetView<SecondController> {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: RaisedButton(
          onPressed: () {
            Get.back();
          },
          child: Text('Back to First from ${controller.title.value}'),
        ),
      ),
    );
  }
}

class ThirdScreen extends GetView<ThirdController> {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: RaisedButton(
          onPressed: () {
            Get.offNamed('/first');
          },
          child: Text('Off to First ${controller.title.value}'),
        ),
      ),
    );
  }
}
tneotia commented 4 years ago

Okay, sorry about that - thought the code would work. Thanks for confirming!

roipeker commented 4 years ago

If it's working, i will close this issue.