Browser should store up-to-date state in browser history entry when using router

[chunhtai]

a spin off of the comment #65777 (comment)

Currently the browser history does not update the current state even if router report different state as long as the url does not change. We should makes sure it will update the current state when that happens.

For example if the router sends the following RouteInformation updates:

1. RouteInformation(location: '/user', state: 'state1')
2. RouteInformation(location: '/user', state: 'state2')
3. RouteInformation(location: '/user', state: 'state3')
4. RouteInformation(location: 'setting', state: 'state4')

The browser history should store the following entries:

RouteInformation(location: '/user', state: 'state3') -> RouteInformation(location: 'setting', state: 'state4')

But current implementation of browser history instead store the following:

RouteInformation(location: '/user', state: 'state1') -> RouteInformation(location: 'setting', state: 'state4')

Repro

import 'package:flutter/cupertino.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
void main() {
  runApp(
    MaterialApp.router(
      routeInformationParser: SimpleParser(),
      routerDelegate: CounterRouterDelegate(),
    ),
  );
}

class SimpleParser extends RouteInformationParser<RouteInformation> {
  Future<RouteInformation> parseRouteInformation(RouteInformation routeInformation){
    return SynchronousFuture<RouteInformation>(routeInformation);
  }
  RouteInformation restoreRouteInformation(RouteInformation routeInformation){
    return routeInformation;
  }
}

class CounterRouterDelegate extends RouterDelegate<RouteInformation> with ChangeNotifier, PopNavigatorRouterDelegateMixin<RouteInformation> {
  @override
  final GlobalKey<NavigatorState> navigatorKey = GlobalKey<NavigatorState>();
  @override
  RouteInformation get currentConfiguration => _currentConfiguration;
  late RouteInformation _currentConfiguration;
  set currentConfiguration(RouteInformation value) {
    _currentConfiguration = value;
    notifyListeners();
  }
  @override
  Future<void> setNewRoutePath(RouteInformation configuration) {
    _currentConfiguration = configuration;
    return SynchronousFuture<void>(null);
  }

  @override
  Widget build(BuildContext context){
    return Navigator(
      key: navigatorKey,
      pages: [
        MaterialPage(child: CounterApp(_currentConfiguration.state as int?)),
        if (_currentConfiguration.location != '/')
          MaterialPage(child: AnotherScreen())
      ],
      onPopPage: (Route<dynamic> route, dynamic result) {
        currentConfiguration = RouteInformation(
          location: '/',
          state: currentConfiguration.state,
        );
        return route.didPop(result);
      },
    );
  }
}

class CounterApp extends StatelessWidget {
  CounterApp(this.count) : super();
  final int? count;
  @override
  Widget build(BuildContext context){
    return Scaffold(
      appBar:  AppBar(actions: [IconButton(onPressed: (){
        CounterRouterDelegate delegate = Router.of(context).routerDelegate as CounterRouterDelegate;
        delegate.currentConfiguration = RouteInformation(
          location: '/another',
          state: delegate.currentConfiguration.state,
        );
      }, icon: Icon(Icons.circle))],),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: [
            Text('You have pushed the button this many times:'),
            Text('${count ?? 0}', style: Theme.of(context).textTheme.display1),
          ],
        )
      ),
      floatingActionButton: FloatingActionButton(
        onPressed: () {
          CounterRouterDelegate delegate = Router.of(context).routerDelegate as CounterRouterDelegate;
          int count = delegate.currentConfiguration.state as int? ?? 0;

          delegate.currentConfiguration = RouteInformation(
            location: delegate.currentConfiguration.location,
            state: count + 1,
          );
        },
        child: Icon(Icons.add),
      ),
    );
  }
}

class AnotherScreen extends StatelessWidget {
  AnotherScreen() : super();
  @override
  Widget build(BuildContext context){
    return Scaffold(
      appBar:  AppBar(),
      body: Center(
          child: Column(
            mainAxisAlignment: MainAxisAlignment.center,
            children: [
              Text('Another screen'),
            ],
          )
      ),
    );
  }
}

steps:

1. click the add button
2. click the circle icon on top right to push a new page
3. press browser backward button

expect: the counter stay the same
actual: counter is reseted to 0

[chunhtai]

@mdebbar does the new behavior looks good to you?

In the real world app, if they stores scroll position in the RouteInformation.state, when they go to a different page and press browser back button. The should see the latest scroll position instead of the scroll position when they first enter the page.

[mdebbar]

@chunhtai yeah, that makes sense. Is this going to require a new platform message to replace state?

[chunhtai]

@chunhtai yeah, that makes sense. Is this going to require a new platform message to replace state?

Or we can extend our existing platform method routerUpdated to take an additional flag to represent whether it is a replace or push. Is there a preference?

[mdebbar]

I'm assuming you mean routeInformationUpdated. No preference. I think it's easier to just add a flag to the existing platform method.

[tomgilder]

@chunhtai the PR for this has broken back button navigation on all of Routemaster's example apps.

In Chrome, the back button is no longer enabled after any navigation.

I haven't had time to check what's going on yet, but might be worth reverting?

[tomgilder]

@chunhtai also checked beamer's examples, and it's the same - back button doesn't function. Examples work fine with current Flutter Dev 2.5.0-5.0.pre.

cc @slovnicki

[chunhtai]

Hi @tomgilder,

I tried running your app, and it keeps spliting out

Error: Expected a value of type 'String?', but got one of type '_JsonMap'
    at Object.throw_ [as throw] (http://localhost:62458/dart_sdk.js:5087:11)
    at Object.castError (http://localhost:62458/dart_sdk.js:5046:15)
    at Object.cast [as as] (http://localhost:62458/dart_sdk.js:5371:17)
    at dart.NullableType.new.as (http://localhost:62458/dart_sdk.js:6874:60)
    at Object.JsonExtensions$124tryString [as JsonExtensions|tryString] (http://localhost:62458/dart_sdk.js:181075:25)
    at _engine.EngineSingletonFlutterWindow.new.<anonymous> (http://localhost:62458/dart_sdk.js:176611:168)
    at Generator.next (<anonymous>)
    at runBody (http://localhost:62458/dart_sdk.js:38671:34)
    at Object._async [as async] (http://localhost:62458/dart_sdk.js:38702:7)
    at http://localhost:62458/dart_sdk.js:176586:57
    at _engine.EngineSingletonFlutterWindow.new._waitInTheLine (http://localhost:62458/dart_sdk.js:176577:27)
    at _waitInTheLine.next (<anonymous>)
    at http://localhost:62458/dart_sdk.js:38652:33
    at _RootZone.runUnary (http://localhost:62458/dart_sdk.js:38523:59)
    at _FutureListener.thenAwait.handleValue (http://localhost:62458/dart_sdk.js:33725:29)
    at handleValueCallback (http://localhost:62458/dart_sdk.js:34277:49)
    at Function._propagateToListeners (http://localhost:62458/dart_sdk.js:34315:17)
    at _Future.new.[_completeWithValue] (http://localhost:62458/dart_sdk.js:34163:23)
    at async._AsyncCallbackEntry.new.callback (http://localhost:62458/dart_sdk.js:34184:35)
    at Object._microtaskLoop (http://localhost:62458/dart_sdk.js:38790:13)
    at _startMicrotaskLoop (http://localhost:62458/dart_sdk.js:38796:13)
    at http://localhost:62458/dart_sdk.js:34531:9

which points to this line https://github.com/flutter/engine/blob/95c7ed2b5d7865c8e6ac07ae76020bf4f1a4d349/lib/web_ui/lib/src/engine/window.dart#L173
it said some of the parameter should be string but was actually a jsonmap. Can't really tell what was going on.

I tried example in https://medium.com/flutter/learning-flutters-new-navigation-and-routing-system-7c9068155ade, the backbutton works fine there

[chunhtai]

The root cause is this flutter/engine@596571bcd1

[chunhtai]

Filed #87823

[slovnicki]

@tomgilder @chunhtai

I tried example in https://medium.com/flutter/learning-flutters-new-navigation-and-routing-system-7c9068155ade, the backbutton works fine there

I just want to add that this example is working fine probably because routeInformationParser leaves the RouteInformation.state to be null:
https://gist.github.com/johnpryan/5ce79aee5b5f83cfababa97c9cf0a204#file-main-dart-L49

  @override
  RouteInformation restoreRouteInformation(BookRoutePath path) {
    if (path.isHomePage) {
      return RouteInformation(location: '/');
    }
    if (path.isDetailsPage) {
      return RouteInformation(location: '/book/${path.id}');
    }
    return null;
  }

[chunhtai]

@slovnicki That is right, It is fixed in the engine side. Right now, just need to wait for the engine roll

[github-actions]

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.