fix(vue): tabs no longer get unmounted when navigating back to a tabs context

[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?

Attempting to modify the .ion-page-hidden class for the transitioning element (typically the leaving element), causes an occasional JavaScript exception to throw when the element is already destroyed.

Issue Number: Resolves #24332

What is the new behavior?

Aligns the Vue implementation for IonRouterOutlet against additional safety checks that exist in the React implementation.

Does this introduce a breaking change?

• Yes
• No

Other information

[sean-perkins]

@liamdebeasi would be cool if we could sneak this into our patch release for v5. There's no functional behavior side effects, just wrapping of logic in appropriate bound checks.

I've confirmed with the reproduction app in the issue (you'll have to remove their @fortawesome private NPM packages in the package.json if you want to confirm as well).

Downside of this bug existing, is the number of junk exceptions it'll log in implementations making use of an error logging service like Sentry.

[liamdebeasi]

I don't think this change fixes the underlying issue. There appear to be two issues here:

1. The Home tab is getting unmounted when it should not be.
2. When going back from a non-tabs page to a tab page, the inner router outlet (inside of ion-tabs) thinks we are also transitioning from the Home page to the More page. In this case, there should be no transition because it is the outlet outlet that resolves the issue.

I would prefer we resolve these two issues as opposed to patching over them.

I found some concrete steps to reproduce the issue:

1. Load app. You start on /more.
2. Click the "Home" tab. You are now on /home.
3. Click the "Sessions" tab. You are now on /sessions.
4. Click the "More" tab. You are now on /more.
5. Click "Terms of Use". You are now on /agreement-terms.
6. Click the ion-back-button. You are now on /more again, but the Home tab has been unmounted.
7. Click "Privacy Policy". You are now on /agreements-privacy.
8. Click the ion-back-button. You are now on /more again, but there is the classList error because the inner router outlet is trying to transition from the Home tab to the More tab, but the Home tab has been unmounted.

I think if we can figure out why the inner router outlet is transitioning in the first place, then we should be able to fix both of these issues.

[liamdebeasi]

The defect seems to be around here: https://github.com/ionic-team/ionic-framework/blob/main/packages/vue/src/components/IonRouterOutlet.ts#L79-L84

Outlet 2 (the tabs outlet) is calling setupViewItem even when previousMatchedRouteRef is undefined. Doing some digging, but adding an undefined check here may fix the issue. That ain't it

[liamdebeasi]

The big issue here is that Home is being unmounted when leaving a non-tab page and coming back to a tab page.

Here is an E2E test I wrote we can use to verify the fixed behavior:

it('should not unmount tab 1 when leaving tabs context', () => {
    cy.visit('http://localhost:8080/tabs');
    cy.ionPageVisible('tab1');

    // Dynamically add tab 4 because tab 3 redirects to tab 1
    cy.get('#add-tab').click();

    cy.get('ion-tab-button#tab-button-tab4').click();
    cy.ionPageHidden('tab1');
    cy.ionPageVisible('tab4');

    cy.get('ion-tab-button#tab-button-tab2').click();
    cy.ionPageHidden('tab4');
    cy.ionPageVisible('tab2');

    cy.get('[data-pageid="tab2"] #routing').click();
    cy.ionPageVisible('routing');
    cy.ionPageHidden('tabs');

    cy.ionBackClick('routing');
    cy.ionPageDoesNotExist('routing');
    cy.ionPageVisible('tabs');
    cy.ionPageVisible('tab2');
    cy.ionPageHidden('tab1');
  });

[liamdebeasi]

Another thing to try is maybe we can check to see if the currentMatchedRouteRef matches whatever the route for the associated active view is: https://github.com/ionic-team/ionic-framework/blob/main/packages/vue/src/components/IonRouterOutlet.ts#L83

Note this is different than comparing to previousMatchedRouteRef as this value will be undefined when navigating away from tabs (even though the active tab inside of tabs will still be considered active)

[liamdebeasi]

This is from another fix I did, but I noticed I copy and pasted the reference issue incorrectly

[sean-perkins]

This looks good on my end 👍 verified with the reproduction app that the error is no longer thrown.

[averyjohnston]

Looks good to me as well 👍 We may want to update the PR description with the new behavior.