Locations are keeping past BeamLocation.history even after "disposed"

[matuella]

Describe the bug
After coming back to a previously navigated BeamLocation (meaning that it was disposed from the beamingHistory stack), it still keeps its BeamLocation.history state stored. It seems that the instance is never disposed/re-created.

Beamer version: 1.0.0

To Reproduce

Minimal reproducible code

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

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

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    final delegate =
      BeamerDelegate(initialPath: '/a', locationBuilder: BeamerLocationBuilder(beamLocations: [LocationA(), LocationB()]));

    return BeamerProvider(
      routerDelegate: delegate,
      child: MaterialApp.router(
        routerDelegate: delegate,
        routeInformationParser: BeamerParser(),
      ),
    );
  }
}

class PageWithTitle extends StatelessWidget {
  PageWithTitle(this.title, this.children);
  final String title;
  final List<Widget> children;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(title)
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: children,
        ),
      ),
    );
  }
}


class LocationA extends BeamLocation<BeamState> {
  @override
  List<BeamPage> buildPages(BuildContext context, BeamState state) => [
    BeamPage(key: ValueKey('a'), child: PageWithTitle('LocationA - /a', [
      ElevatedButton(onPressed: () => Beamer.of(context).beamToNamed('/b'), child: Text('beam to /b')),
      ElevatedButton(onPressed: () => Beamer.of(context).beamToNamed('/alt-b'), child: Text('beam to /alt-b')),
    ])),
  ];

  @override
  List<Pattern> get pathPatterns => ['/a'];
}

class LocationB extends BeamLocation<BeamState> {
  @override
  List<BeamPage> buildPages(BuildContext context, BeamState state) {
    final locationHistory = Text('Current Location History: ${Beamer.of(context).currentBeamLocation.history.map((e) => e.routeInformation.location)}');
    final beamBack = ElevatedButton(onPressed: () => Beamer.of(context).beamBack(), child: Text('beamBack'));
    return [
      if (state.pathPatternSegments.contains('b')) 
        BeamPage(key: ValueKey('b'), child: PageWithTitle('LocationB - /b', [beamBack, locationHistory])),
      if (state.pathPatternSegments.contains('alt-b'))
        BeamPage(key: ValueKey('alt-b'), child: PageWithTitle('LocationB - /alt-b', [beamBack, locationHistory])),
    ];
  }

  @override
  List<Pattern> get pathPatterns => ['/b', '/alt-b'];
}

To reproduce the behavior, using the code above:

1. Press the beam to /b button, then beamBack.
2. Press the beam to /alt-b button, then beamBack.
3. You'll see that, instead of popping to /a, we will back to /b page, which now exists in the history stack.

The same can be done if navigated to alt-b, then to b. It doesn't matter, the "history" of that particular stack is preserved, even after it's dismissed.

Expected behavior
Expected that, after the BeamLocation was disposed, all of its history should be "cleared".

On 0.14.1 it was working properly - a BeamLocation that was "disposed" never stored previous states, I've tested the code above to certify of that.

Working in any OS.

Additional context
I've tried to track down this bug and it seems that it's related to BeamerLocationBuilder and how it deals with BeamLocation.isCurrent, which is a new behavior added in 1.0.0. If you debug this isCurrent, after we beamBack from /b to /a, the LocationB, never gets its isCurrent property updated to false, and the current LocationA also isn't updated isCurrent to true, which makes me think that beamer goes into an inconsistent state.

So, related debugged functions that seems odd to me are: BeamLocation.create (which is used in Utils.chooseBeamLocation) and _addToBeamingHistory, which handles the isCurrent behavior. All of which are related to BeamerLocationBuilder.

[slovnicki]

Hey @matuella
I see what you mean, indeed. Thanks for a nice explanation and reproducible example 🙏

And if we were to use

locationBuilder: (ri, _) {
  if (ri.location?.contains('b') == true) {
    return LocationB()..create(ri);
  }
  return LocationA()..create(ri);
},

then there is no issue as BeamLocations are recreated with create.

I feel like we should not discard the history lightly, but if we wanted to; we would be doing it in BeamLocation.disposeState and putting @mustCallSuper annotation. Yeah, we should also put setting the isCurrent there and similarly in BeamLocation.initState.

Let me think about history for a while and get back to you with some conclusions. Don't get me wrong, your example definitely shows the undesired behavior. I want to produce some scenarios where keeping the history like that is desired and think of another way to solve the problem which seems more like a bug in beamBack, i.e. the incompatibility of current history structure with its needs. beamBack would be better off using some flat List of history.

[matuella]

Working as intended right now! Thanks a lot!