Skip to content

fix(react): Nav unmounts component while invoking popTo or popToRoot #27821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/react/src/framework-delegate.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import type { FrameworkDelegate } from '@ionic/core/components';
import { createPortal } from 'react-dom';

import { generateId } from './utils/generateId';

// TODO(FW-2959): types

type ReactComponent = (props?: any) => JSX.Element;
Expand All @@ -10,6 +12,9 @@ export const ReactDelegate = (
removeView: (view: React.ReactElement) => void
): FrameworkDelegate => {
const refMap = new WeakMap<HTMLElement, React.ReactElement>();
const reactDelegateId = `react-delegate-${generateId()}`;
// Incrementing counter to generate unique keys for each view
let id = 0;

const attachViewToDom = async (
parentElement: HTMLElement,
Expand All @@ -22,7 +27,8 @@ export const ReactDelegate = (
parentElement.appendChild(div);

const componentWithProps = component(propsOrDataObj);
const hostComponent = createPortal(componentWithProps, div);
const key = `${reactDelegateId}-${id++}`;
const hostComponent = createPortal(componentWithProps, div, key);

refMap.set(div, hostComponent);

Expand Down
23 changes: 16 additions & 7 deletions packages/react/test/base/src/pages/navigation/NavComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
IonBackButton,
IonPage,
} from '@ionic/react';
import React, { useRef } from 'react';
import React, { useEffect, useRef } from 'react';

const PageOne = ({
nav,
Expand Down Expand Up @@ -39,7 +39,10 @@ const PageOne = ({
<IonNavLink
routerDirection="forward"
component={PageTwo}
componentProps={{ someValue: 'Hello' }}
componentProps={{
someValue: 'Hello',
nav: nav,
}}
>
<IonButton>Go to Page Two</IonButton>
</IonNavLink>
Expand All @@ -48,7 +51,7 @@ const PageOne = ({
);
};

const PageTwo = (props?: { someValue: string }) => {
const PageTwo = ({ nav, ...rest }: { someValue: string; nav: React.MutableRefObject<HTMLIonNavElement> }) => {
return (
<>
<IonHeader>
Expand All @@ -61,16 +64,21 @@ const PageTwo = (props?: { someValue: string }) => {
</IonHeader>
<IonContent id="pageTwoContent">
<IonLabel>Page two content</IonLabel>
<div id="pageTwoProps">{JSON.stringify(props)}</div>
<IonNavLink routerDirection="forward" component={PageThree}>
<div id="pageTwoProps">{JSON.stringify(rest)}</div>
<IonNavLink routerDirection="forward" component={() => <PageThree nav={nav} />}>
<IonButton>Go to Page Three</IonButton>
</IonNavLink>
</IonContent>
</>
);
};

const PageThree = () => {
const PageThree = ({ nav }: { nav: React.MutableRefObject<HTMLIonNavElement> }) => {
useEffect(() => {
return () => {
window.dispatchEvent(new CustomEvent('pageThreeUnmounted'));
};
});
return (
<>
<IonHeader>
Expand All @@ -81,8 +89,9 @@ const PageThree = () => {
</IonButtons>
</IonToolbar>
</IonHeader>
<IonContent>
<IonContent id="pageThreeContent">
<IonLabel>Page three content</IonLabel>
<IonButton onClick={() => nav.current.popToRoot()}>popToRoot</IonButton>
</IonContent>
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,23 @@ describe('IonNav', () => {
cy.get('#pageTwoProps').should('have.text', '{"someValue":"Hello"}');
});

it('should unmount pages when popping to root', () => {
// Issue: https://github.com/ionic-team/ionic-framework/issues/27798

cy.contains('Go to Page Two').click();
cy.get('#pageTwoContent').should('be.visible');

cy.contains('Go to Page Three').click();
cy.get('#pageThreeContent').should('be.visible');

cy.window().then((window) => {
window.addEventListener('pageThreeUnmounted', cy.stub().as('pageThreeUnmounted'));
});

cy.get('ion-button').contains('popToRoot').click();
cy.get('#pageThreeContent').should('not.exist');

cy.get('@pageThreeUnmounted').should('have.been.calledOnce');
});

});