From 8c8fe17d5c21da2559cc811c7c3718754a4f0b8b Mon Sep 17 00:00:00 2001 From: zhbhun Date: Wed, 19 Jul 2023 11:58:55 +0800 Subject: [PATCH 1/2] fix(nav): Component mount only once while invoking popTo or popToRoot of IonNav --- packages/react/src/components/navigation/IonNav.tsx | 5 +++-- packages/react/src/framework-delegate.tsx | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/react/src/components/navigation/IonNav.tsx b/packages/react/src/components/navigation/IonNav.tsx index 8d2ca03e3ae..c7fd97f3b22 100644 --- a/packages/react/src/components/navigation/IonNav.tsx +++ b/packages/react/src/components/navigation/IonNav.tsx @@ -1,6 +1,6 @@ import type { FrameworkDelegate, JSX } from '@ionic/core/components'; import { defineCustomElement } from '@ionic/core/components/ion-nav.js'; -import React, { useMemo, useState } from 'react'; +import React, { useMemo, useRef, useState } from 'react'; import { ReactDelegate } from '../../framework-delegate'; import { createReactComponent } from '../react-component-lib'; @@ -19,6 +19,7 @@ type IonNavProps = JSX.IonNav & { // eslint-disable-next-line @typescript-eslint/no-unused-vars const IonNavInternal: React.FC = ({ children, forwardedRef, ...restOfProps }) => { + const idRef = useRef(0); const [views, setViews] = useState([]); /** @@ -28,7 +29,7 @@ const IonNavInternal: React.FC = ({ children, forwardedRef, ...rest const addView = (view: React.ReactElement) => setViews((existingViews) => [...existingViews, view]); const removeView = (view: React.ReactElement) => setViews((existingViews) => existingViews.filter((v) => v !== view)); - const delegate = useMemo(() => ReactDelegate(addView, removeView), []); + const delegate = useMemo(() => ReactDelegate(addView, removeView, () => idRef.current++), []); return ( diff --git a/packages/react/src/framework-delegate.tsx b/packages/react/src/framework-delegate.tsx index dcb4823ef00..b44c8ff0c11 100644 --- a/packages/react/src/framework-delegate.tsx +++ b/packages/react/src/framework-delegate.tsx @@ -7,7 +7,8 @@ type ReactComponent = (props?: any) => JSX.Element; export const ReactDelegate = ( addView: (view: React.ReactElement) => void, - removeView: (view: React.ReactElement) => void + removeView: (view: React.ReactElement) => void, + nextID?: () => number ): FrameworkDelegate => { const refMap = new WeakMap(); @@ -22,7 +23,7 @@ export const ReactDelegate = ( parentElement.appendChild(div); const componentWithProps = component(propsOrDataObj); - const hostComponent = createPortal(componentWithProps, div); + const hostComponent = createPortal(componentWithProps, div, nextID?.()); refMap.set(div, hostComponent); From aa8fb13a9d344c3fa65b131ed12df6e543ba774d Mon Sep 17 00:00:00 2001 From: Sean Perkins Date: Tue, 19 Sep 2023 17:12:53 -0400 Subject: [PATCH 2/2] fix(react): nav unmounts react components --- .../src/components/navigation/IonNav.tsx | 5 ++-- packages/react/src/framework-delegate.tsx | 11 ++++++--- .../src/pages/navigation/NavComponent.tsx | 23 +++++++++++++------ .../tests/e2e/specs/navigation/IonNav.cy.ts | 19 +++++++++++++++ 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/packages/react/src/components/navigation/IonNav.tsx b/packages/react/src/components/navigation/IonNav.tsx index c7fd97f3b22..8d2ca03e3ae 100644 --- a/packages/react/src/components/navigation/IonNav.tsx +++ b/packages/react/src/components/navigation/IonNav.tsx @@ -1,6 +1,6 @@ import type { FrameworkDelegate, JSX } from '@ionic/core/components'; import { defineCustomElement } from '@ionic/core/components/ion-nav.js'; -import React, { useMemo, useRef, useState } from 'react'; +import React, { useMemo, useState } from 'react'; import { ReactDelegate } from '../../framework-delegate'; import { createReactComponent } from '../react-component-lib'; @@ -19,7 +19,6 @@ type IonNavProps = JSX.IonNav & { // eslint-disable-next-line @typescript-eslint/no-unused-vars const IonNavInternal: React.FC = ({ children, forwardedRef, ...restOfProps }) => { - const idRef = useRef(0); const [views, setViews] = useState([]); /** @@ -29,7 +28,7 @@ const IonNavInternal: React.FC = ({ children, forwardedRef, ...rest const addView = (view: React.ReactElement) => setViews((existingViews) => [...existingViews, view]); const removeView = (view: React.ReactElement) => setViews((existingViews) => existingViews.filter((v) => v !== view)); - const delegate = useMemo(() => ReactDelegate(addView, removeView, () => idRef.current++), []); + const delegate = useMemo(() => ReactDelegate(addView, removeView), []); return ( diff --git a/packages/react/src/framework-delegate.tsx b/packages/react/src/framework-delegate.tsx index b44c8ff0c11..7379b4b97e0 100644 --- a/packages/react/src/framework-delegate.tsx +++ b/packages/react/src/framework-delegate.tsx @@ -1,16 +1,20 @@ 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; export const ReactDelegate = ( addView: (view: React.ReactElement) => void, - removeView: (view: React.ReactElement) => void, - nextID?: () => number + removeView: (view: React.ReactElement) => void ): FrameworkDelegate => { const refMap = new WeakMap(); + const reactDelegateId = `react-delegate-${generateId()}`; + // Incrementing counter to generate unique keys for each view + let id = 0; const attachViewToDom = async ( parentElement: HTMLElement, @@ -23,7 +27,8 @@ export const ReactDelegate = ( parentElement.appendChild(div); const componentWithProps = component(propsOrDataObj); - const hostComponent = createPortal(componentWithProps, div, nextID?.()); + const key = `${reactDelegateId}-${id++}`; + const hostComponent = createPortal(componentWithProps, div, key); refMap.set(div, hostComponent); diff --git a/packages/react/test/base/src/pages/navigation/NavComponent.tsx b/packages/react/test/base/src/pages/navigation/NavComponent.tsx index 96e4b14af0a..17275465af1 100644 --- a/packages/react/test/base/src/pages/navigation/NavComponent.tsx +++ b/packages/react/test/base/src/pages/navigation/NavComponent.tsx @@ -11,7 +11,7 @@ import { IonBackButton, IonPage, } from '@ionic/react'; -import React, { useRef } from 'react'; +import React, { useEffect, useRef } from 'react'; const PageOne = ({ nav, @@ -39,7 +39,10 @@ const PageOne = ({ Go to Page Two @@ -48,7 +51,7 @@ const PageOne = ({ ); }; -const PageTwo = (props?: { someValue: string }) => { +const PageTwo = ({ nav, ...rest }: { someValue: string; nav: React.MutableRefObject }) => { return ( <> @@ -61,8 +64,8 @@ const PageTwo = (props?: { someValue: string }) => { Page two content -
{JSON.stringify(props)}
- +
{JSON.stringify(rest)}
+ }> Go to Page Three @@ -70,7 +73,12 @@ const PageTwo = (props?: { someValue: string }) => { ); }; -const PageThree = () => { +const PageThree = ({ nav }: { nav: React.MutableRefObject }) => { + useEffect(() => { + return () => { + window.dispatchEvent(new CustomEvent('pageThreeUnmounted')); + }; + }); return ( <> @@ -81,8 +89,9 @@ const PageThree = () => { - + Page three content + nav.current.popToRoot()}>popToRoot ); diff --git a/packages/react/test/base/tests/e2e/specs/navigation/IonNav.cy.ts b/packages/react/test/base/tests/e2e/specs/navigation/IonNav.cy.ts index e0940bd3b95..3298abf0b89 100644 --- a/packages/react/test/base/tests/e2e/specs/navigation/IonNav.cy.ts +++ b/packages/react/test/base/tests/e2e/specs/navigation/IonNav.cy.ts @@ -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'); + }); + });