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

[go_router] push and pushReplacement are not in the correct ShellRoute #120665

Closed
ValentinVignal opened this issue Feb 14, 2023 · 10 comments · Fixed by flutter/packages#5497
Closed
Assignees
Labels
found in release: 3.7 Found to occur in 3.7 found in release: 3.8 Found to occur in 3.8 has reproducible steps The issue has been confirmed reproducible and is ready to work on p: go_router The go_router package P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels. r: fixed Issue is closed as already fixed in a newer version team-go_router Owned by Go Router team triaged-go_router Triaged by Go Router team

Comments

@ValentinVignal
Copy link
Contributor

ValentinVignal commented Feb 14, 2023

Steps to Reproduce

  1. Execute flutter run on the code sample (see "Code sample" section below)
    There are 4 pages. 2 of them are in a ShellRoute while the 2 others are outside
  2. Navigate with go, notice that everything works fine
  3. Navigate with pushand/or pushReplacement.
  4. Notice that the ShellRoute is off (You can see a "Shell" blue box on tap). When pushing a page, the "Shell" blue box doesn't disappear/appear when it should, it remains in the current state (shown or hidden).

Expected results:

When pushing to a route

  • Inside the shell: I expect to see the blue box "Shell"
  • Outside the shell: I expect to not see the blue box "Shell"

Actual results:

The blue box "Shell" remains hidden/shown and is not updated.

Code sample

Or you can checkout https://github.com/ValentinVignal/flutter_app_stable/tree/go-router/push-and-push-replacement-do-not-update-shell-route

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

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

final router = GoRouter(
  initialLocation: '/out-shell-0',
  routes: [
    GoRoute(
      path: '/out-shell-0',
      builder: (context, state) {
        return const Screen(name: 'Out shell 0');
      },
    ),
    GoRoute(
      path: '/out-shell-1',
      builder: (context, state) {
        return const Screen(name: 'Out shell 1');
      },
    ),
    ShellRoute(
      routes: [
        GoRoute(
          path: '/in-shell-0',
          builder: (context, state) {
            return const Screen(name: 'In shell 0');
          },
        ),
        GoRoute(
          path: '/in-shell-1',
          builder: (context, state) {
            return const Screen(name: 'In shell 1');
          },
        ),
      ],
      builder: (context, state, child) {
        return Shell(child: child);
      },
    ),
  ],
);

class Shell extends StatelessWidget {
  const Shell({
    required this.child,
    super.key,
  });

  final Widget child;

  @override
  Widget build(BuildContext context) {
    return Column(
      crossAxisAlignment: CrossAxisAlignment.start,
      children: [
        const Material(
          color: Colors.blue,
          child: Text('In Shell'),
        ),
        Expanded(
          child: child,
        ),
      ],
    );
  }
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp.router(
      routerConfig: router,
    );
  }
}

class Screen extends StatelessWidget {
  const Screen({
    required this.name,
    super.key,
  });

  final String name;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: ListView(
        children: [
          Text(name),
          const _Tile(route: '/in-shell-0'),
          const _Tile(route: '/in-shell-1'),
          const _Tile(route: '/out-shell-0'),
          const _Tile(route: '/out-shell-1'),
        ],
      ),
    );
  }
}

class _Tile extends StatelessWidget {
  const _Tile({
    required this.route,
    Key? key,
  }) : super(key: key);

  final String route;

  @override
  Widget build(BuildContext context) {
    final router = GoRouter.of(context);
    return ListTile(
      title: Text(route),
      trailing: Row(
        mainAxisSize: MainAxisSize.min,
        children: [
          TextButton(
            onPressed: () {
              router.go(route);
            },
            child: const Text('go'),
          ),
          TextButton(
            onPressed: () {
              router.push(route);
            },
            child: const Text('push'),
          ),
          TextButton(
            onPressed: () {
              router.pushReplacement(route);
            },
            child: const Text('pushReplacement'),
          ),
        ],
      ),
    );
  }
}
Logs
Launching lib/main.dart on Chrome in debug mode...
Waiting for connection from debug service on Chrome...             34.1s
This app is linked to the debug service: ws://127.0.0.1:62518/h8E-lMlKvWY=/ws
Debug service listening on ws://127.0.0.1:62518/h8E-lMlKvWY=/ws

💪 Running with sound null safety 💪

🔥  To hot restart changes while running, press "r" or "R".
For a more detailed help message, press "h". To quit, press "q".

An Observatory debugger and profiler on Chrome is available at: http://127.0.0.1:62518/h8E-lMlKvWY=
The Flutter DevTools debugger and profiler on Chrome is available at: http://127.0.0.1:9101?uri=http://127.0.0.1:62518/h8E-lMlKvWY=

Application finished.
Analyzing flutter_app_stable...                                         
No issues found! (ran in 15.1s)
[✓] Flutter (Channel stable, 3.7.3, on macOS 11.6.8 20G730 darwin-x64, locale en-GB)
    • Flutter version 3.7.3 on channel stable at /Users/valentin/flutter/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 9944297138 (5 days ago), 2023-02-08 15:46:04 -0800
    • Engine revision 248290d6d5
    • Dart version 2.19.2
    • DevTools version 2.20.1

[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.3)
    • Android SDK at /usr/local/Caskroom/android-sdk/4333796
    • Platform android-33, build-tools 30.0.3
    • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.11+0-b60-7590822)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 13.2.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 13C100
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2021.1)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.11+0-b60-7590822)

[✓] VS Code (version 1.75.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.58.0

[✓] Connected device (2 available)
    • macOS (desktop) • macos  • darwin-x64     • macOS 11.6.8 20G730 darwin-x64
    • Chrome (web)    • chrome • web-javascript • Google Chrome 109.0.5414.119

[✓] HTTP Host Availability
    • All required HTTP hosts are available

• No issues found!
Video
Screen.Recording.2023-02-14.at.2.39.51.PM.mov
@danagbemava-nc danagbemava-nc added the in triage Presently being triaged by the triage team label Feb 14, 2023
@danagbemava-nc
Copy link
Member

Hi @ValentinVignal, does the video below capture the issue clearly? (Your video wasn't uploaded, unfortunately)

video
Screen.Recording.2023-02-14.at.10.49.30.mov

@danagbemava-nc danagbemava-nc added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Feb 14, 2023
@ValentinVignal
Copy link
Contributor Author

ValentinVignal commented Feb 15, 2023

Your video wasn't uploaded, unfortunately

Oh no, sorry for that. I edited the issue to re-upload it.

does the video below capture the issue clearly

But yes, your video shows the issue clearly, thanks!

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Feb 15, 2023
@danagbemava-nc
Copy link
Member

Thanks for the confirmation.

The issue is reproducible using the code sample in #120665 (comment)

Labeling for further investigation.

flutter doctor -v
[✓] Flutter (Channel stable, 3.7.3, on macOS 13.1 22C65 darwin-arm64, locale en-GB)
    • Flutter version 3.7.3 on channel stable at /Users/nexus/dev/sdks/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 9944297138 (6 days ago), 2023-02-08 15:46:04 -0800
    • Engine revision 248290d6d5
    • Dart version 2.19.2
    • DevTools version 2.20.1

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
    • Android SDK at /Users/nexus/Library/Android/sdk
    • Platform android-33, build-tools 33.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.15+0-b2043.56-8887301)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.2)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14C18
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[!] Android Studio (version 2022.1)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    ✗ Unable to find bundled Java version.
    • Try updating or re-installing Android Studio.

[!] Android Studio (version 2022.1)
    • Android Studio at /Users/nexus/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/221.6008.13.2211.9477386/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    ✗ Unable to find bundled Java version.
    • Try updating or re-installing Android Studio.

[✓] IntelliJ IDEA Community Edition (version 2022.3.2)
    • IntelliJ at /Applications/IntelliJ IDEA CE.app
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart

[✓] VS Code (version 1.75.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.58.0

[✓] Connected device (3 available)
    • M2007J20CG (mobile) • adb-5dd3be00-17AYzd._adb-tls-connect._tcp. • android-arm64  • Android 12 (API 31)
    • macOS (desktop)     • macos                                      • darwin-arm64   • macOS 13.1 22C65 darwin-arm64
    • Chrome (web)        • chrome                                     • web-javascript • Google Chrome 110.0.5481.77

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 2 categories.
[!] Flutter (Channel master, 3.8.0-12.0.pre.67, on macOS 13.1 22C65 darwin-arm64, locale en-GB)
    • Flutter version 3.8.0-12.0.pre.67 on channel master at /Users/nexus/dev/sdks/flutters
    ! Warning: `flutter` on your path resolves to /Users/nexus/dev/sdks/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/nexus/dev/sdks/flutters. Consider adding /Users/nexus/dev/sdks/flutters/bin to the front of your path.
    ! Warning: `dart` on your path resolves to /Users/nexus/dev/sdks/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /Users/nexus/dev/sdks/flutters. Consider adding /Users/nexus/dev/sdks/flutters/bin to the front of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision fd01812f67 (4 hours ago), 2023-02-14 18:43:38 -0800
    • Engine revision d860892528
    • Dart version 3.0.0 (build 3.0.0-233.0.dev)
    • DevTools version 2.21.1
    • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades.

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
    • Android SDK at /Users/nexus/Library/Android/sdk
    • Platform android-33, build-tools 33.0.0
    • Java binary at: /Users/nexus/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/221.6008.13.2211.9477386/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.15+0-b2043.56-8887301)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.2)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14C18
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2022.1)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.15+0-b2043.56-8887301)

[✓] Android Studio (version 2022.1)
    • Android Studio at /Users/nexus/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/221.6008.13.2211.9477386/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.15+0-b2043.56-8887301)

[✓] IntelliJ IDEA Community Edition (version 2022.3.2)
    • IntelliJ at /Applications/IntelliJ IDEA CE.app
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart

[✓] VS Code (version 1.75.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.58.0

[✓] Connected device (3 available)
    • M2007J20CG (mobile) • adb-5dd3be00-17AYzd._adb-tls-connect._tcp. • android-arm64  • Android 12 (API 31)
    • macOS (desktop)     • macos                                      • darwin-arm64   • macOS 13.1 22C65 darwin-arm64
    • Chrome (web)        • chrome                                     • web-javascript • Google Chrome 110.0.5481.77

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 1 category.

@danagbemava-nc danagbemava-nc added package flutter/packages repository. See also p: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on p: go_router The go_router package found in release: 3.7 Found to occur in 3.7 found in release: 3.8 Found to occur in 3.8 and removed in triage Presently being triaged by the triage team labels Feb 15, 2023
@johnpryan
Copy link
Contributor

This could be related to other issues with push() and ShellRoute: #111842

@JohnF17
Copy link

JohnF17 commented Sep 22, 2023

Thanks to this comment by @patrikheinonen, I think your issue @valentinvigna, might just be solved as well, try this code below

Btw the blue box appears at the very top right, due to no safeArea widget.

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

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

// private root navigator
final _rootNavigatorKey = GlobalKey<NavigatorState>();

final router = GoRouter(
  initialLocation: '/out-shell-0',
  navigatorKey: _rootNavigatorKey,
  routes: [
    GoRoute(
      parentNavigatorKey: _rootNavigatorKey,
      path: '/out-shell-0',
      builder: (context, state) {
        return const Screen(name: 'Out shell 0');
      },
    ),
    GoRoute(
      parentNavigatorKey: _rootNavigatorKey,
      path: '/out-shell-1',
      builder: (context, state) {
        return const Screen(name: 'Out shell 1');
      },
    ),
    ShellRoute(
      parentNavigatorKey: _rootNavigatorKey,
      routes: [
        GoRoute(
          path: '/in-shell-0',
          builder: (context, state) {
            return const Screen(name: 'In shell 0');
          },
        ),
        GoRoute(
          path: '/in-shell-1',
          builder: (context, state) {
            return const Screen(name: 'In shell 1');
          },
        ),
      ],
      builder: (context, state, child) {
        return Shell(child: child);
      },
    ),
  ],
);

class Shell extends StatelessWidget {
  const Shell({
    required this.child,
    super.key,
  });

  final Widget child;

  @override
  Widget build(BuildContext context) {
    return Column(
      crossAxisAlignment: CrossAxisAlignment.start,
      children: [
        const Material(
          color: Colors.blue,
          child: Text('In Shell'),
        ),
        Expanded(
          child: child,
        ),
      ],
    );
  }
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp.router(
      routerConfig: router,
    );
  }
}

class Screen extends StatelessWidget {
  const Screen({
    required this.name,
    super.key,
  });

  final String name;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: ListView(
        children: [
          Text(name),
          const _Tile(route: '/in-shell-0'),
          const _Tile(route: '/in-shell-1'),
          const _Tile(route: '/out-shell-0'),
          const _Tile(route: '/out-shell-1'),
        ],
      ),
    );
  }
}

class _Tile extends StatelessWidget {
  const _Tile({
    required this.route,
    Key? key,
  }) : super(key: key);

  final String route;

  @override
  Widget build(BuildContext context) {
    final router = GoRouter.of(context);
    return ListTile(
      title: Text(route),
      trailing: Row(
        mainAxisSize: MainAxisSize.min,
        children: [
          TextButton(
            onPressed: () {
              router.go(route);
            },
            child: const Text('go'),
          ),
          TextButton(
            onPressed: () {
              router.push(route);
            },
            child: const Text('push'),
          ),
          TextButton(
            onPressed: () {
              router.pushReplacement(route);
            },
            child: const Text('pushReplacement'),
          ),
        ],
      ),
    );
  }
}

@ValentinVignal
Copy link
Contributor Author

@JohnF17 I shouldn't have to specify the root navigator key, I think this issue should remain open.

Thank you for the work around though

@JohnF17
Copy link

JohnF17 commented Sep 22, 2023

Thank you for the work around though

Glad it could help, though @ValentinVignal, might I ask why is using a root navigator looked down upon?, i've had others say this as well 👀.

@ValentinVignal
Copy link
Contributor Author

ValentinVignal commented Sep 22, 2023

Glad it could help, though @ValentinVignal, might I ask why is using a root navigator looked down upon?, i've had others say this as well 👀.

It is redundant. When creating the GoRoute architecture. It is implied that a GoRouter will use the navigator of its closest parent (root navigator or navigator from its closest shell route). So specifying parentNavigatorKey: _rootNavigatorKey to the GoRoute with the path: '/out-shell-0', is redundant since it is already just below the root navigator

@JohnF17
Copy link

JohnF17 commented Sep 22, 2023

Hmm that is true indeed, GoRouter has to use the navigator of its closest parent by default, besides being redundant though it hasn't got any performance issues I believe, so this workaround might be best till this issue gets addressed.
As far as we know the issue lingers around 'navigator inheritance' and lets hope this workaround helps maintainers see the root cause.

P.s. I've got a question around when to use StatefulShellRoute and ShellRoute, any articles or stackoverflow posts or if anyone would like to explain, please do.

@danagbemava-nc danagbemava-nc added the r: fixed Issue is closed as already fixed in a newer version label Dec 22, 2023
ChopinDavid pushed a commit to wwt/flutter-packages that referenced this issue Dec 28, 2023
This pr refactor RouteMatchList to be a tree structure.

Added a common base class RouteMatchBase. It is extended by both RouteMatch and ShellRouteMatch.

The RouteMatch is for GoRoute, and is always a leaf node

The ShellRouteMatch is for ShellRouteBase, and is always and intermediate node with a list of child RouteMatchBase[s].

This pr also redo how push is processed. Will add a doc explain this shortly.

This is a breaking change, will write a migration guide soon.

fixes flutter/flutter#134524
fixes flutter/flutter#130406
fixes flutter/flutter#126365
fixes flutter/flutter#125752
fixes flutter/flutter#120791
fixes flutter/flutter#120665
fixes flutter/flutter#113001
fixes flutter/flutter#110512
Copy link

github-actions bot commented Jan 5, 2024

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 5, 2024
arc-yong pushed a commit to Arctuition/packages-arc that referenced this issue Jun 14, 2024
This pr refactor RouteMatchList to be a tree structure.

Added a common base class RouteMatchBase. It is extended by both RouteMatch and ShellRouteMatch.

The RouteMatch is for GoRoute, and is always a leaf node

The ShellRouteMatch is for ShellRouteBase, and is always and intermediate node with a list of child RouteMatchBase[s].

This pr also redo how push is processed. Will add a doc explain this shortly.

This is a breaking change, will write a migration guide soon.

fixes flutter/flutter#134524
fixes flutter/flutter#130406
fixes flutter/flutter#126365
fixes flutter/flutter#125752
fixes flutter/flutter#120791
fixes flutter/flutter#120665
fixes flutter/flutter#113001
fixes flutter/flutter#110512
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
found in release: 3.7 Found to occur in 3.7 found in release: 3.8 Found to occur in 3.8 has reproducible steps The issue has been confirmed reproducible and is ready to work on p: go_router The go_router package P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels. r: fixed Issue is closed as already fixed in a newer version team-go_router Owned by Go Router team triaged-go_router Triaged by Go Router team
Projects
No open projects
Status: No status
Development

Successfully merging a pull request may close this issue.

6 participants