Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RouterOutlet navigation problem #944

Open
EdsonMello-code opened this issue Mar 14, 2024 · 7 comments
Open

RouterOutlet navigation problem #944

EdsonMello-code opened this issue Mar 14, 2024 · 7 comments
Labels
new New issue request attention

Comments

@EdsonMello-code
Copy link

Describe the bug
Navigation is not working correctly, because when I navigate to the RouterOutlet and navigate to another separate module, the parent module of the RouterOutlet maintains the binds and also the module itself as shown in the example to reproduce the error.

Environment
image

To Reproduce

import 'dart:developer';

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

void main() {
  return runApp(ModularApp(module: AppModule(), child: const AppWidget()));
}

class AppWidget extends StatelessWidget {
  const AppWidget({super.key});

  @override
  Widget build(BuildContext context) {
    Modular.setInitialRoute('/home/page1');

    return MaterialApp.router(
      title: 'My Smart App',
      theme: ThemeData(primarySwatch: Colors.blue),
      routerConfig: Modular.routerConfig,
    );
  }
}

class AppModule extends Module {
  @override
  void routes(r) {
    r.module('/', module: LoginModule());
    r.module('/home', module: HomeModule());
  }
}

class InternalPage extends StatelessWidget {
  final String title;
  final Color color;
  const InternalPage({super.key, required this.title, required this.color});

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      floatingActionButton: FloatingActionButton(
        onPressed: () {
          Modular.get<CounterController>().increment();
          Modular.to.navigate('/');
        },
      ),
      body: Material(
        color: color,
        child: Center(child: Text(title)),
      ),
    );
  }
}

class HomeModule extends Module {
  @override
  void binds(Injector i) {
    i.addLazySingleton<CounterController>(CounterController.new,
        config: BindConfig(
      onDispose: (value) {
        log('CounterController disposed');
      },
    ));
  }

  @override
  void routes(RouteManager r) {
    r.child('/', child: (context) => const HomePage(), children: [
      ChildRoute('/page1',
          child: (context) =>
              const InternalPage(title: 'page 1', color: Colors.red)),
      ChildRoute('/page2',
          child: (context) =>
              const InternalPage(title: 'page 2', color: Colors.amber)),
      ChildRoute('/page3',
          child: (context) =>
              const InternalPage(title: 'page 3', color: Colors.green)),
    ]);
  }
}

class CounterController {
  int value = 0;
  void increment() {
    value++;
    log('CounterController: $value');
  }
}

class HomePage extends StatelessWidget {
  const HomePage({super.key});

  @override
  Widget build(BuildContext context) {
    final leading = SizedBox(
      width: MediaQuery.of(context).size.width * 0.3,
      child: Column(
        children: [
          ListTile(
            title: const Text('Page 1'),
            onTap: () => Modular.to.navigate('/home/page1'),
          ),
          ListTile(
            title: const Text('Page 2'),
            onTap: () => Modular.to.navigate('/home/page2'),
          ),
          ListTile(
            title: const Text('Page 3'),
            onTap: () => Modular.to.navigate('/home/page3'),
          ),
        ],
      ),
    );

    return Scaffold(
      appBar: AppBar(title: const Text('Home Page')),
      body: Row(
        children: [
          leading,
          Container(width: 2, color: Colors.black45),
          const Expanded(child: RouterOutlet()),
        ],
      ),
    );
  }
}

class LoginModule extends Module {
  @override
  void routes(RouteManager r) {
    r.child('/', child: (context) {
      return const LoginPage();
    });
  }
}

class LoginPage extends StatelessWidget {
  const LoginPage({super.key});

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      floatingActionButton: FloatingActionButton(
        onPressed: () {
          Modular.to.navigate('/home/');
        },
        child: const Icon(Icons.local_gas_station_outlined),
      ),
    );
  }
}

Expected behavior
I expect the CounterController bind to be destroyed as well as the HomeModule module when I navigate to another module (LoginModule)

Screenshots
image

@EdsonMello-code EdsonMello-code added the new New issue request attention label Mar 14, 2024
@RibeiroRibas
Copy link

same issue here.

@DimensaSpaki
Copy link

Same issue

I analyzed the tracker of modular_core code, specifically the reportPushRoute and reportPopRoute to monitor the _disposeTags and the unbindModules.

It is noticeable that if you keep repeating the flow of opening and closing the login module, the routes "/home/" accumulate at moduleTags.
Captura de tela 2024-05-17 185546

This is a problem because the unbindModule is only called when the list of tags is empty, so the module is not disposed.

I changed the remove to a removeWhere to clean up these duplicate routes.

   moduleTags.removeWhere((element) => element == tag);
   if (tag.characters.last == '/') {
     moduleTags.removeWhere((element) => element == '$tag/'.replaceAll('//', ''));
   }

But I need to understand why the routeOutlet adds this extra module tag during navigation.

@eduardoflorence
Copy link

Hi @DimensaSpaki,

Have you tested with the new version 6.3.3? There was a fix of it in her.

@DimensaSpaki
Copy link

Hi @eduardoflorence,

Yes, I tested using the latest version.
I cleared all the cache just to be sure and ran @EdsonMello-code example again and had the same problem. The same issue occurs in the app I am developing.

@eduardoflorence
Copy link

Hi @EdsonMello-code, @RibeiroRibas, @DimensaSpaki
I made the pull request to fix this a few months ago. Last month it was approved and merged, but as the change was in modular_core, it is also necessary to update this package on pub.dev. I already asked @jacobaraujo7 to do this.
If you want to test before the update comes out, just put the excerpt below in pubspec.yaml:

dependency_overrides:
  modular_core:
    git: 
      url: https://github.com/eduardoflorence/modular.git
      ref: module_not_dispose
      path: modular_core

@DimensaSpaki
Copy link

@eduardoflorence, the example worked, but I think there is still another problem.

In my app the bind didn't disposed, but my scenario is more complex than the example, there is a RouterOutlet inside another RouterOutlet and also redirect routes.

Later, I will try to adapt the example and check if I can reproduce the problem. I also want to monitor what happens in this piece of code that was modified running my app.

Thank you for the reply :)

@eduardoflorence
Copy link

@EdsonMello-code, @RibeiroRibas, @DimensaSpaki
I discovered another problem and have already put a new PR to be approved, please update the overrides to:

dependency_overrides:
  modular_core:
    git:
      url: https://github.com/eduardoflorence/modular.git
      ref: bug-create-exported-injector
      path: modular_core

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new New issue request attention
Projects
None yet
Development

No branches or pull requests

4 participants