csells / go_router

The purpose of the go_router for Flutter is to use declarative routes to reduce complexity, regardless of the platform you're targeting (mobile, web, desktop), handling deep linking from Android, iOS and the web while still allowing an easy-to-use developer experience.
https://gorouter.dev
441 stars 97 forks source link

update docs for guidance on integration w/ the navigator #105

Closed Andrew-Bekhiet closed 3 years ago

Andrew-Bekhiet commented 3 years ago

So I started migrating my app to use go_router but I can't find the equivalent of this: final result = await Navigator.of(context).pushNamed('SomeRoute')

Any ideas how to do similar thing in go_router?

csells commented 3 years ago

You can use the normal Navigator for that. It works just fine with GoRouter.

Andrew-Bekhiet commented 3 years ago

It doesn't At first I thought I might have done something wrong because I was using a global navigator key, but I made a minimal reproducible example below and it throws an exception

Please correct me if I'm doing something wrong or I misunderstood something in go_router

Exception:

════════ Exception caught by gesture ═══════════════════════════════════════════
The following assertion was thrown while handling a gesture:
Navigator.onGenerateRoute was null, but the route named "/OtherPage" was referenced.

To use the Navigator API with named routes (pushNamed, pushReplacementNamed, or pushNamedAndRemoveUntil), the Navigator must be provided with an onGenerateRoute handler.
The Navigator was:
  NavigatorState#5caf9(tickers: tracking 1 ticker)
When the exception was thrown, this was the stack
#0      NavigatorState._routeNamed.<anonymous closure>
package:flutter/…/widgets/navigator.dart:4171
#1      NavigatorState._routeNamed
package:flutter/…/widgets/navigator.dart:4181
#2      NavigatorState.pushNamed
package:flutter/…/widgets/navigator.dart:4243
#3      _MyHomePageState.build.<anonymous closure>
package:go_router_test/main.dart:88
#4      _InkResponseState._handleTap
package:flutter/…/material/ink_well.dart:989
#5      GestureRecognizer.invokeCallback
package:flutter/…/gestures/recognizer.dart:193
#6      TapGestureRecognizer.handleTapUp
package:flutter/…/gestures/tap.dart:608
#7      BaseTapGestureRecognizer._checkUp
package:flutter/…/gestures/tap.dart:296
#8      BaseTapGestureRecognizer.handlePrimaryPointer
package:flutter/…/gestures/tap.dart:230
#9      PrimaryPointerGestureRecognizer.handleEvent
package:flutter/…/gestures/recognizer.dart:558
#10     PointerRouter._dispatch
package:flutter/…/gestures/pointer_router.dart:94
#11     PointerRouter._dispatchEventToRoutes.<anonymous closure>
package:flutter/…/gestures/pointer_router.dart:139
#12     _LinkedHashMapMixin.forEach (dart:collection-patch/compact_hash.dart:400:8)
#13     PointerRouter._dispatchEventToRoutes
package:flutter/…/gestures/pointer_router.dart:137
#14     PointerRouter.route
package:flutter/…/gestures/pointer_router.dart:123
#15     GestureBinding.handleEvent
package:flutter/…/gestures/binding.dart:440
#16     GestureBinding.dispatchEvent
package:flutter/…/gestures/binding.dart:420
#17     RendererBinding.dispatchEvent
package:flutter/…/rendering/binding.dart:278
#18     GestureBinding._handlePointerEventImmediately
package:flutter/…/gestures/binding.dart:374
#19     GestureBinding.handlePointerEvent
package:flutter/…/gestures/binding.dart:338
#20     GestureBinding._flushPointerEventQueue
package:flutter/…/gestures/binding.dart:296
#21     GestureBinding._handlePointerDataPacket
package:flutter/…/gestures/binding.dart:279
#25     _invoke1 (dart:ui/hooks.dart:185:10)
#26     PlatformDispatcher._dispatchPointerDataPacket (dart:ui/platform_dispatcher.dart:293:7)
#27     _dispatchPointerDataPacket (dart:ui/hooks.dart:98:31)
(elided 3 frames from dart:async)
Handler: "onTap"
Recognizer: TapGestureRecognizer#c0a88
    debugOwner: GestureDetector
    state: possible
    won arena
    finalPosition: Offset(148.8, 535.9)
    finalLocalPosition: Offset(24.6, 15.5)
    button: 1
    sent tap down
════════════════════════════════════════════════════════════════════════════════

Minimal reproducible example:

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

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

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

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    final router = GoRouter(
      routes: [
        GoRoute(
          path: '/',
          name: '/',
          pageBuilder: (context, state) => MaterialPage(
            key: state.pageKey,
            child: const MyHomePage(title: 'Some Title'),
          ),
        ),
        GoRoute(
          path: '/OtherPage',
          name: '/OtherPage',
          pageBuilder: (context, state) => MaterialPage(
            key: state.pageKey,
            child: const Scaffold(
              body: Center(
                child: Text('Other Page'),
              ),
            ),
          ),
        ),
      ],
      errorPageBuilder: (context, state) => MaterialPage(
        key: state.pageKey,
        child: const Scaffold(
          body: Text('Oops! an error ocurred'),
        ),
      ),
    );

    return MaterialApp.router(
      title: 'Go Router test',
      routeInformationParser: router.routeInformationParser,
      routerDelegate: router.routerDelegate,
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({Key? key, required this.title}) : super(key: key);

  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  int _counter = 0;

  void _incrementCounter() {
    setState(() {
      _counter++;
    });
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(widget.title),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            const Text(
              'You have pushed the button this many times:',
            ),
            Text(
              '$_counter',
              style: Theme.of(context).textTheme.headline4,
            ),
            ElevatedButton(
              onPressed: () {
                Navigator.of(context).pushNamed('/OtherPage');
              },
              child: const Text('Go to other page'),
            ),
          ],
        ),
      ),
      floatingActionButton: FloatingActionButton(
        onPressed: _incrementCounter,
        tooltip: 'Increment',
        child: const Icon(Icons.add),
      ),
    );
  }
}
csells commented 3 years ago

Sorry. I was a little loosey goosey when it came to "it works just fine." The MaterialApp.router constructor doesn't take an onGenerateRoute callback, so there's no way for me to map Navigator.pushNamed to the set of GoRouter names. However, you can use Navigator.push and Navigator.pop if you'd like to get results from a route. It won't affect anything wrt the current location as far as GoRouter is concerned or update the browser's address bar, but that doesn't make sense when you've got a call stack waiting on a result anyway.

I should really add some docs on how to mix and match GoRouter w/ the build in Navigator.

Andrew-Bekhiet commented 3 years ago

Ok good, but it would also be nice if GoRouter can support something like this await GoRouter.of(context).goAndWaitForResult('/SomeGoRoute')

csells commented 3 years ago

Hmmm... That's a nice feature request actually, since I can't provide an onGenerateRoute but you still want to use the GoRouter set of paths and names. However, the location and browser's address bar wouldn't be updated, which takes it outside of the realm of what GoRouter is for. Perhaps something like final res = await Navigator.push(context, context.pageForName('foo')) would make the most sense.

csells commented 3 years ago

fixed in v2.2.0 via new navigator integration guidance

I don't believe it makes sense to allow for results from GoRouter.push or GoRouter.go nor do I think it makes sense to add entries to the routing table just for the purpose of pulling them out for Navigator integration, since the pages won't be suitable for normal GoRouter navigation.

@Andrew-Bekhiet take a look and let me know what you think

Andrew-Bekhiet commented 3 years ago

Ok this lgtm and I think if this package is intended for deep linking and representing navigation stack in a url then yes it doesn't make sense to make it wait for result from another page

And if anyone really wants to wait for results from another page or make logic on the initial page itself, then I think they can use the query parameters or the extra object to communicate between pages, though I don't think this is a good practice, and the initial page must be StatefulWidget to take advantage of this

For example:

/// Initial Page
void goToSecondPage() {
  GoRouter.of(context).go('/SecondPage');
}

@override
void didChangeDependencies() {
  super.didChangeDependencies();

  if(widget.previousResult != null) {
    // process logic here
  }
}

/// Second Page
void finish() {
  GoRouter.of(context).go('/InitialPage', extra: someResult);
}