Don't retain parameters not passed as query string parameters

[PureWeen]

Description of Change

Currently when you pass extra parameters to shell via the parameters dictionary those are retained forever and reapplied with each navigation. The workarounds to remove those parameters is fairly cumbersome. Through discussions with users, it's become apparent that we need a mechanism to have parameters that only get applied during the initial navigation request. This PR clears any parameters that aren't part of the QueryString.

It's particularly problematic to retain those parameters because if the user passes a complex object that object is retained in memory forever unknown to the user.

FAQ

• Why is the Query Property an internal BP?
  • This was done for Hot Reload purposes. In theory there would be scenarios where hot reload could trigger a page to be recreated and thus all BP's would need to get copied from one Page to another.
• What about a perfect solution?
• Ideally we restructure shell when there's more time to flesh out more interfaces/apis for parameters passing. For now, we need a way for users to opt out of parameters being reapplied
• What changed between MAUI and XF?
  • The only thing that changed was the addition of the dictionary overload on GotoAsync. Parameters have always reapplied. So, for users coming from XF there is no change in behavior. This would mainly be a change for users that started using the new API.
• What if users are using the setting of the parameter for OnAppearing?
  • IApplyQueryAttributes will still get called so users can use that interface.

Breaking Change

• When renavigating to a page (clicking back or navigating tabs) ApplyQueryAttributes will only be called with the parameters that are part of the query string of that route. This also means that properties defined in QueryPropertyAttribute will only be set if they are part of the query string.

Examples for the docs to improve clarity

We should enhance the navigation part of the Shell docs to include various example permutations of how parameters are handled.

Still works the same

await GotoAsync("firstPage?param=1");
await GotoAsync("nextPage");
await GotoAsync(".."); // param = 1 will get applied to `firstPage`

Breaking Change

var parameter = new ShellRouteParameters
{
     { "ComplexObject", obj}
};

await GotoAsync("firstPage?param=1", parameter);
await GotoAsync("nextPage");
await GotoAsync(".."); // param = 1 will get applied to `firstPage` but `ComplexObject` won't

var parameter = new ShellRouteParameters
{
     { "ComplexObject", obj}
};

await GotoAsync("firstPage", parameter);
await GotoAsync("nextPage");
await GotoAsync(".."); // IQueryAttributable.ApplyQueryAttributes will still get called but the dict will be empty

Issues Fixed

Fixes #10294

Alternative Approaches

Add an enumeration to Shell so users can control the life cycle.

public enum ParameterPassingBehavior
{
    OnlyPassOnFirstNavigation,
    PassEveryTime
}

Issues

• Complex objects are still retained inside the BP which might cause unknown memory leaks for users.

Add a new overload for shell GotoAsync that differentiates navigation parameters vs query parameters.

GotoAsync("url", new NavigationArgs())

class NavigationArgs : IReadOnlyDictionary<string, object>
{
    // Additional properties we might use later on that help with other scenarios.
     public bool IsAnimated
     public bool PopToRoot
}

Issues

• Adding too much API that we might deprecate down the road

[ghost]

🚨 API change(s) detected @davidbritch FYI

[bakerhillpins]

@PureWeen
What you have seems clear enough to me as far as the proposal goes.

For my part in all of this it still seems odd to retain any parameters since the uri that's being applied to navigate back via GotoAsync in your examples, specifically "..", doesn't actually contain any parameters. I guess I never felt comfortable with my understanding of what is/was the intended feature design to begin with. Was it to represent the application page Z-order within a URI or was it to behave like a browser back button combined with URIs.

[espenrl]

Thanks for creating this PR and inviting to comment.

In my opinion I find it odd to retain parameters from previous navigations, neither from uri or dictionary. If a developer would need to access parameters from previous navigations that could easily be solved by storing them in a field in the first place. That's what my code does currently.

I also refrain from using messenger pattern to communicate between viewmodels, I higly prefer to use parameter passing.

Perhaps the logic behind the current implementation is connected to rehydrating pages? Maybe instances are destroyed and need to be created again from scratch? And then would need all previous parameters to create a fresh instance. I don't know too much about how that works. I use DI singleton viewmodel instances mostly so that would never be a problem I would see (if a page is destroyed and recreated it would get the same viewmodel instance).