Skip to content

fix(react): type overlay hook component props - #31183

Closed
SSDWGG wants to merge 1 commit into
ionic-team:mainfrom
SSDWGG:fix/react-overlay-hook-prop-types
Closed

fix(react): type overlay hook component props#31183
SSDWGG wants to merge 1 commit into
ionic-team:mainfrom
SSDWGG:fix/react-overlay-hook-prop-types

Conversation

@SSDWGG

@SSDWGG SSDWGG commented May 30, 2026

Copy link
Copy Markdown

Issue number: resolves #28680


What is the current behavior?

useIonModal and useIonPopover accept componentProps as any, so props passed to typed React components are not checked.

What is the new behavior?

  • Adds a generic ReactComponent<Props = any> helper while keeping ReactComponentOrElement backward-compatible.
  • Adds overloads for useIonModal and useIonPopover so component props are inferred from function/class components.
  • Keeps JSX element usage permissive for compatibility.
  • Adds type coverage for valid props, missing required props, and unknown props.

Does this introduce a breaking change?

  • Yes
  • No

The generic defaults to any, and JSX element overloads keep the previous permissive behavior.

Testing

  • npm ci in packages/react
  • npm run test.spec -- --runTestsByPath src/hooks/__tests__/overlay-hook-types.spec.tsx
  • npx tsc --noEmit --project tsconfig.json --skipLibCheck
  • npx prettier --check src/hooks/useIonModal.ts src/hooks/useIonPopover.ts src/hooks/__tests__/overlay-hook-types.spec.tsx src/models/ReactComponentOrElement.ts
  • npx eslint src/hooks/useIonModal.ts src/hooks/useIonPopover.ts src/hooks/__tests__/overlay-hook-types.spec.tsx src/models/ReactComponentOrElement.ts

Note: plain npx tsc --noEmit --project tsconfig.json is blocked by the existing AbortSignal declaration conflict between DOM and @types/node; the same command passes with --skipLibCheck. ESLint exits successfully, with the repository ignore warning for src/hooks/__tests__/overlay-hook-types.spec.tsx.

@SSDWGG
SSDWGG requested a review from a team as a code owner May 30, 2026 03:14
@SSDWGG
SSDWGG requested a review from OS-jacobbell May 30, 2026 03:14
@vercel

vercel Bot commented May 30, 2026

Copy link
Copy Markdown

@SSDWGG is attempting to deploy a commit to the Ionic Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added the package: react @ionic/react package label May 30, 2026
@ShaneK

ShaneK commented Aug 16, 2026

Copy link
Copy Markdown
Member

Hey @SSDWGG! Thanks for picking this up, and sorry for the slow reply.

The direction is right, but we went with a different implementation, so I'm going to close this one.

The componentProps argument stays optional, so a component that declares required props still type checks when you pass nothing at all, which is the most common way to get this wrong. It's the same gap the review on #29058 settled. The signature also infers the props type from componentProps, which makes the self-referencing dismiss pattern circular, so it fails with TS7022. Both v9 controller examples in our docs use that pattern, so they would stop compiling.

I opened a replacement against major-9.0, which is also where this needs to go rather than main, as we concluded on #29158, though this may not actually be able to be merged until v10 due to how close we are to v9 and it being a breaking change. You're a co-author on the new PR along with @aeharding and @liamdebeasi, since it builds on all three attempts at solving this issue. Thanks again for your contribution!

@ShaneK ShaneK closed this Aug 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: react @ionic/react package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Improved types for react useIonModal

2 participants