fix(router): pop to previous route with params

[sean-perkins]

Pull request checklist

Please check if your PR fulfills the following requirements:

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
• Build (npm run build) was run locally and any changes were pushed
• Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe):

What is the current behavior?

The function used to determine the correct path to resolve for ion-router does not take route parameters into consideration. It only compares the route ids, with the first definition always taking priority over all other routes. This means that if a router has multiple routes sharing the same component, the first definition will always be the route that is determined when running the matching algorithm.

i.e.:

<ion-route url="/" component="my-page" />
<ion-route url="/:s1" component="my-page" />
<ion-route url="/:s1/:s2" component="my-page" />
<ion-route url="/:s1/:s2/:s3" component="my-page" />

/ will always resolve as the path when determining the route to navigate to when going back to my-page component.

Issue Number: Resolves #24223

What is the new behavior?

When calculating the best matched route in the chain, routes that both match the active route's id (page component match) and match the definition of route params will be weighted higher.

Does this introduce a breaking change?

• Yes
• No

Other information

The behavior before this PR (URL resets on pop or back interaction):

Kapture.2021-12-02.at.22.10.15.mp4

The behavior after this PR:

Kapture.2021-12-02.at.22.07.51.mp4

[averyjohnston]

I had a couple thoughts, but I trust your judgment on whether changes actually need to be made; I might be misunderstanding the logic.

[liamdebeasi]

Other than the Object.keys change, this PR looks good. Can we also change the commit subject to be something more descriptive? Maybe something like fix(router): popping route now accounts for route params.

Might be good to have the issue author verify the fix in their app if they have not already.

Great job!

[averyjohnston]

Ditto 👍

[vicb]

Doesn't this assume that all segments of routeChain.path[j] are parameter (i.e. :param) ?
Would it work if some of the segments are fixed string ? (i.e. /fixed/:s1/:s2)

[vicb]

@sean-perkins Is the test app shown in the "Other information" section of the PR description available somewhere ? Shouldn't it be added as an e2e test ?