Skip to content
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
/**
* Mock the `@ionic/core/components` ESM boundary so Jest can load the wrapper.
* Jest can't import Ionic's ESM-only custom elements, so we stub the few
* values `createInlineOverlayComponent` reaches: `componentOnReady` (used to
* redirect `cachedOriginalParent` after mount) and `getPlatforms`/`isPlatform`
* (pulled in transitively via `./utils`).
*/
jest.mock('@ionic/core/components', () => ({
// Delegates to a mutable hook so individual tests can run side effects
// (e.g. teleporting the host) at the moment the overlay comes online.
componentOnReady: (el: HTMLElement, cb: () => void) => mockComponentOnReady(el, cb),
getPlatforms: () => [],
isPlatform: () => false,
}));

import { act, render } from '@testing-library/react';
import React from 'react';

import { createInlineOverlayComponent } from '../createInlineOverlayComponent';

// Default: resolve immediately, no side effects. Reset in `afterEach`.
// `mock` prefix lets the `jest.mock` factory reference it despite hoisting.
const defaultComponentOnReady = (_el: HTMLElement, cb: () => void) => cb();
let mockComponentOnReady = defaultComponentOnReady;

// Mirror how `IonModal` is generated: a delegate-host overlay.
const IonModal = createInlineOverlayComponent<any, any>('ion-modal', undefined, true) as any;
// An overlay rendered inside another overlay observes the nested context.
const IonPopover = createInlineOverlayComponent<any, any>('ion-popover', undefined, true) as any;

/**
* Simulate what CoreDelegate does when an overlay presents: it teleports the
* host element out of its portal parent into another in-document container
* (the running app uses ion-app; here we use any sibling).
*/
const teleport = (el: HTMLElement) => {
const dest = document.createElement('div');
dest.id = 'teleport-destination';
document.body.appendChild(dest);
dest.appendChild(el);
};

afterEach(() => {
document.body.innerHTML = '';
mockComponentOnReady = defaultComponentOnReady;
});

describe('createInlineOverlayComponent: unmount cleanup', () => {
it('removes a relocated overlay on unmount even when it never opened', () => {
const { unmount } = render(<IonModal />);

const modal = document.body.querySelector('ion-modal') as HTMLElement;
expect(modal).toBeTruthy();

// Overlay is teleported while still closed (isOpen === false).
teleport(modal);

unmount();

expect(document.querySelector('ion-modal')).toBeNull();
});

it('removes a normally-portaled overlay on unmount (no relocation)', () => {
const { unmount } = render(<IonModal />);

expect(document.body.querySelector('ion-modal')).toBeTruthy();

unmount();

expect(document.querySelector('ion-modal')).toBeNull();
});

it('removes an open, relocated overlay on unmount', () => {
const { unmount } = render(<IonModal />);
const modal = document.body.querySelector('ion-modal') as HTMLElement;

// Drive the overlay to its open state the way core does, then teleport it.
act(() => {
modal.dispatchEvent(new CustomEvent('willPresent'));
});
teleport(modal);

unmount();

expect(document.querySelector('ion-modal')).toBeNull();
});

it('does not orphan a relocated overlay across a StrictMode mount/unmount cycle', () => {
/**
* React 18 StrictMode mounts, unmounts, then remounts each component in
* dev to surface unsafe state reuse. CoreDelegate teleports the host out
* of its portal parent as the overlay comes online (simulated here from
* componentOnReady, which fires in componentDidMount). At the first
* StrictMode unmount the host is relocated but still closed - exactly the
* case that previously left an orphan behind, producing a duplicate
* `<ion-modal>` in the DOM.
*/
mockComponentOnReady = (el, cb) => {
teleport(el);
cb();
};

render(
<React.StrictMode>
<IonModal />
</React.StrictMode>
);

// Only the surviving remount's host should remain. The discarded first
// mount must not leave an orphan.
expect(document.querySelectorAll('ion-modal')).toHaveLength(1);
});

it('removes a relocated nested overlay on unmount even when it never opened', () => {
// keepContentsMounted renders the children (and the nested popover) while
// the outer modal is closed, so the popover observes the nested context.
const { unmount } = render(
<IonModal keepContentsMounted={true}>
<IonPopover />
</IonModal>
);

const popover = document.body.querySelector('ion-popover') as HTMLElement;
expect(popover).toBeTruthy();

// Nested overlay is teleported while still closed.
teleport(popover);

unmount();

expect(document.querySelector('ion-popover')).toBeNull();
});

it('removes an open, relocated nested overlay on unmount', () => {
const { unmount } = render(
<IonModal keepContentsMounted={true}>
<IonPopover />
</IonModal>
);

const popover = document.body.querySelector('ion-popover') as HTMLElement;

// Drive the nested popover open the way core does, then teleport it out of
// its `<template>`. This exercises the `node.remove()` branch followed by
// the `isOpen` teardown block on an already-detached node.
act(() => {
popover.dispatchEvent(new CustomEvent('willPresent'));
});
teleport(popover);

expect(() => unmount()).not.toThrow();

expect(document.querySelector('ion-popover')).toBeNull();
});
});
70 changes: 45 additions & 25 deletions packages/react/src/components/createInlineOverlayComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,41 @@ export const createInlineOverlayComponent = <PropType, ElementType>(
componentWillUnmount() {
this.isUnmounted = true;
const node = this.ref.current;
if (!node) {
return;
}
/**
* CoreDelegate (or user code in onWillPresent) can move the overlay out
* of where React rendered it. React's unmount only removes the node from
* its original React location, so we recover a relocated host here,
* regardless of open state.
*
* We can't gate this on `isOpen`: the overlay can be moved before it
* finishes presenting (e.g. the React 18 StrictMode mount/unmount cycle,
* where the present events that flip `isOpen` haven't fired yet), which
* would orphan the relocated host in the DOM. It also has to run
* synchronously here, since React's portal `removeChild` runs after
* `componentWillUnmount` returns and needs the host where it expects it.
*/
if (node.isConnected) {
if (this.props.isNested) {
/**
* Nested overlays render inline inside a `<template>`. If the host
* has been moved out of that template, React's unmount won't reach
* it, so remove it directly. A host still in its template is left
* for React to remove.
*/
if (!(node.parentElement instanceof HTMLTemplateElement)) {
node.remove();
}
} else if (this.portalTarget && node.parentNode !== this.portalTarget) {
/**
* Portaled overlays: move the host back into `portalTarget` so
* React's portal `removeChild` can find it.
*/
this.portalTarget.appendChild(node);
}
}
/**
* If the overlay is being unmounted, but is still
* open, this means the unmount was triggered outside
Expand All @@ -140,30 +175,13 @@ export const createInlineOverlayComponent = <PropType, ElementType>(
* Unmounting the overlay at this stage should skip
* the dismiss lifecycle, including skipping the transition.
*
* Detach the local event listener that performs the state updates,
* before dismissing the overlay, to prevent the callback handlers
* executing after the component has been unmounted. This is to
* avoid memory leaks.
*/
if (node && this.state.isOpen) {
/**
* Detach the local event listener that performs the state updates,
* before dismissing the overlay, to prevent the callback handlers
* executing after the component has been unmounted. This is to
* avoid memory leaks.
*/
if (this.state.isOpen) {
node.removeEventListener('didDismiss', this.handleDidDismiss);
if (this.props.isNested) {
/**
* Nested overlays render inline (no portal). CoreDelegate may
* have moved the node out of its React parent, so React's
* unmount won't reach it. Remove it directly.
*/
node.remove();
} else if (node.isConnected && this.portalTarget && node.parentNode !== this.portalTarget) {
/**
* Portaled path: move the overlay back into `portalTarget` so
* React's portal removeChild can find it. CoreDelegate (or user
* code in onWillPresent) may have moved it elsewhere while open.
*/
this.portalTarget.appendChild(node);
}
detachProps(node, this.props);
}
}
Expand Down Expand Up @@ -288,10 +306,12 @@ export const createInlineOverlayComponent = <PropType, ElementType>(
};

// Forward the nesting context as a prop to avoid contextType on the class.
// The render function is passed via `children` (not as a varargs child) so it
// matches `Context.Consumer`'s render-prop signature `(value) => ReactNode`.
const ReactComponentWithNesting: React.FC<IonicReactInternalProps<PropType>> = (props) =>
createElement(NestedOverlayContext.Consumer, null, (isNested: boolean) =>
createElement(ReactComponent, { ...(props as InternalProps), isNested })
);
createElement(NestedOverlayContext.Consumer, {
children: (isNested: boolean) => createElement(ReactComponent, { ...(props as InternalProps), isNested }),
});
ReactComponentWithNesting.displayName = displayName;

return createForwardRef<PropType, ElementType>(ReactComponentWithNesting, displayName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,16 @@ export const createForwardRef = <PropType, ElementType>(ReactComponent: any, dis
};
forwardRef.displayName = displayName;

return React.forwardRef(forwardRef);
// Cast the render function to the type React.forwardRef already infers for it.
// React 18's `forwardRef` wraps the props in `PropsWithoutRef`, and since
// `PropType` is unconstrained TypeScript can't prove the round-trip is safe.
// The cast keeps the inferred component type intact without widening to `any`.
return React.forwardRef(
forwardRef as React.ForwardRefRenderFunction<
ElementType,
React.PropsWithoutRef<StencilReactExternalProps<PropType, ElementType>>
>
);
};

export const defineCustomElement = (tagName: string, customElement: any) => {
Expand Down
11 changes: 10 additions & 1 deletion packages/react/src/components/utils/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,16 @@ export const createForwardRef = <PropType, ElementType>(
};
forwardRef.displayName = displayName;

return React.forwardRef(forwardRef);
// Cast the render function to the type React.forwardRef already infers for it.
// React 18's `forwardRef` wraps the props in `PropsWithoutRef`, and since
// `PropType` is unconstrained TypeScript can't prove the round-trip is safe.
// The cast keeps the inferred component type intact without widening to `any`.
return React.forwardRef(
forwardRef as React.ForwardRefRenderFunction<
ElementType,
React.PropsWithoutRef<IonicReactExternalProps<PropType, ElementType>>
>
);
};

export const isPlatform = (platform: Platforms) => {
Expand Down
Loading