diff --git a/BREAKING.md b/BREAKING.md index 19194cc658b..93baeb27511 100644 --- a/BREAKING.md +++ b/BREAKING.md @@ -466,6 +466,30 @@ The `@ionic/react-router` package now requires React Router v6. React Router v5 | react-router | 6.4.0+ | | react-router-dom | 6.4.0+ | +**TypeScript** + +The `@ionic/react` package now requires TypeScript 5.4 or later. Its type definitions use `NoInfer`, which TypeScript added in 5.4. This matches the minimum that `@ionic/angular` already requires. + +**Typed Overlay Hook Props** + +The `useIonModal` and `useIonPopover` hooks type `componentProps` against the component they are given, instead of accepting `any`. Props that do not match the component are a compile error, and `componentProps` is required when the component declares required props. Applications passing incorrect props will see new type errors at build time rather than failing at runtime. + +```diff + const Modal: React.FC<{ title: string }> = ({ title }) => {title}; + +- const [present, dismiss] = useIonModal(Modal, { subtitle: 'Wrong' }); ++ const [present, dismiss] = useIonModal(Modal, { title: 'Hello' }); +``` + +Props are read from the component rather than from `componentProps`, so a component declared inline needs its props annotated: + +```diff +- const [present, dismiss] = useIonModal(({ name }) =>
Hello {name}.
, { name: 'Dave' }); ++ const [present, dismiss] = useIonModal(({ name }: { name: string }) =>
Hello {name}.
, { name: 'Dave' }); +``` + +Passing a JSX element rather than a component is unchanged, and `componentProps` is not type checked in that case. + React Router v6 introduces several API changes that will require updates to your application's routing configuration: **Route Definition Changes** diff --git a/packages/migrate/src/migrations/index.ts b/packages/migrate/src/migrations/index.ts index 65ae39e5b78..ebd2d4cf1bf 100644 --- a/packages/migrate/src/migrations/index.ts +++ b/packages/migrate/src/migrations/index.ts @@ -12,6 +12,7 @@ import { angularVersion } from './v9/angular-version.js'; import { angularBrowserPolicy } from './v9/angular-browser-policy.js'; import { angularBrowserPolicyManual } from './v9/angular-browser-policy-manual.js'; import { reactDeps } from './v9/react-deps.js'; +import { reactTypescript } from './v9/react-typescript.js'; import { reactRouter6Routes } from './v9/react-router-6-routes.js'; import { reactRouter6Code } from './v9/react-router-6-code.js'; import { vueDeps } from './v9/vue-deps.js'; @@ -51,6 +52,7 @@ export const allMigrations: Migration[] = [ angularBrowserPolicy, angularBrowserPolicyManual, reactDeps, + reactTypescript, reactRouter6Routes, reactRouter6Code, vueDeps, diff --git a/packages/migrate/src/migrations/v9/react-deps.ts b/packages/migrate/src/migrations/v9/react-deps.ts index b6cc226017d..a1a03d30a9a 100644 --- a/packages/migrate/src/migrations/v9/react-deps.ts +++ b/packages/migrate/src/migrations/v9/react-deps.ts @@ -7,7 +7,7 @@ import { createDepsMigration } from '../../ast/deps-migration.js'; * `react-router-6-code`. * * React is raised only to the 18 floor v9 requires; a newer major is the app's - * call. + * call. TypeScript is handled by `react-typescript`. * * Refer to https://ionicframework.com/docs/updating/9-0#react */ diff --git a/packages/migrate/src/migrations/v9/react-typescript.ts b/packages/migrate/src/migrations/v9/react-typescript.ts new file mode 100644 index 00000000000..3fef0c329fd --- /dev/null +++ b/packages/migrate/src/migrations/v9/react-typescript.ts @@ -0,0 +1,17 @@ +import { createDepsMigration } from '../../ast/deps-migration.js'; + +/** + * Ionic 9 requires TypeScript 5.4 or later. `@ionic/react`'s published types use + * `NoInfer`, which TypeScript added in 5.4. A higher pin is left alone. + * + * Pinned with a caret, unlike the tilde `angular-typescript` uses: React has no + * narrow peer range to satisfy, so any later 5.x is fine. + * + * Refer to https://ionicframework.com/docs/updating/9-0#react-typescript + */ +export const reactTypescript = createDepsMigration({ + id: 'react-typescript', + framework: 'react', + docsUrl: 'https://ionicframework.com/docs/updating/9-0#react-typescript', + bumps: [['typescript', '^5.4.0']], +}); diff --git a/packages/migrate/test/react-typescript.test.ts b/packages/migrate/test/react-typescript.test.ts new file mode 100644 index 00000000000..693e3a5a476 --- /dev/null +++ b/packages/migrate/test/react-typescript.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it } from 'vitest'; + +import { createInMemoryContext } from '../src/context.js'; +import { selectMigrations } from '../src/registry.js'; +import { allMigrations } from '../src/migrations/index.js'; +import { reactTypescript } from '../src/migrations/v9/react-typescript.js'; + +describe('react-typescript', () => { + it('raises TypeScript to the 5.4 floor @ionic/react requires', () => { + const ctx = createInMemoryContext({ + 'package.json': JSON.stringify({ devDependencies: { typescript: '^4.9.5' } }, null, 2), + }); + + reactTypescript.fix!(ctx); + + expect(JSON.parse(ctx.readFile('package.json')!).devDependencies.typescript).toBe('^5.4.0'); + }); + + it('raises a pin declared in dependencies rather than devDependencies', () => { + const ctx = createInMemoryContext({ + 'package.json': JSON.stringify({ dependencies: { typescript: '~5.0.4' } }, null, 2), + }); + + reactTypescript.fix!(ctx); + + expect(JSON.parse(ctx.readFile('package.json')!).dependencies.typescript).toBe('^5.4.0'); + }); + + it('does not downgrade a pin already above the floor', () => { + const ctx = createInMemoryContext({ + 'package.json': JSON.stringify({ devDependencies: { typescript: '^5.9.2' } }, null, 2), + }); + + expect(reactTypescript.detect(ctx)).toEqual([]); + }); + + it('leaves a TypeScript 6 pin alone', () => { + // The caret target is a floor, not a ceiling, so a later major stays put. + const ctx = createInMemoryContext({ + 'package.json': JSON.stringify({ devDependencies: { typescript: '^6.0.0' } }, null, 2), + }); + + expect(reactTypescript.detect(ctx)).toEqual([]); + }); + + it('adds nothing to a project that does not use TypeScript', () => { + const ctx = createInMemoryContext({ + 'package.json': JSON.stringify({ dependencies: { react: '^18.0.0' } }, null, 2), + }); + + reactTypescript.fix!(ctx); + const pkg = JSON.parse(ctx.readFile('package.json')!); + + expect(reactTypescript.detect(ctx)).toEqual([]); + expect(pkg.devDependencies?.typescript).toBeUndefined(); + expect(pkg.dependencies.typescript).toBeUndefined(); + }); + + it('leaves a range it cannot parse alone', () => { + const ctx = createInMemoryContext({ + 'package.json': JSON.stringify({ devDependencies: { typescript: 'catalog:' } }, null, 2), + }); + + expect(reactTypescript.detect(ctx)).toEqual([]); + }); + + it('reports the change it would make', () => { + const ctx = createInMemoryContext({ + 'package.json': JSON.stringify({ devDependencies: { typescript: '^4.9.5' } }, null, 2), + }); + + expect(reactTypescript.detect(ctx)).toEqual([ + { filePath: 'package.json', line: 1, detail: 'set typescript to ^5.4.0' }, + ]); + }); + + it('is selected for a React project and not an Angular one', () => { + const selectedFor = (framework: 'react' | 'angular') => + selectMigrations(allMigrations, { fromMajor: 8, toMajor: 9, frameworks: [framework] }).map((m) => m.id); + + expect(selectedFor('react')).toContain('react-typescript'); + expect(selectedFor('angular')).not.toContain('react-typescript'); + }); +}); diff --git a/packages/react/src/hooks/__tests__/overlay-hook-types.spec.tsx b/packages/react/src/hooks/__tests__/overlay-hook-types.spec.tsx new file mode 100644 index 00000000000..17ad83ef021 --- /dev/null +++ b/packages/react/src/hooks/__tests__/overlay-hook-types.spec.tsx @@ -0,0 +1,153 @@ +import type { ComponentClass, FC, ForwardRefExoticComponent, MemoExoticComponent, RefAttributes } from 'react'; + +import type { useIonModal } from '../useIonModal'; +import type { useIonPopover } from '../useIonPopover'; + +// The hooks are type-only imports, re-declared here. `@ionic/core/components` is ESM +// and Jest runs these specs as CommonJS, so importing them for real fails to load. +declare const useIonModalSignature: typeof useIonModal; +declare const useIonPopoverSignature: typeof useIonPopover; + +interface RequiredProps { + title: string; + count?: number; +} + +interface OptionalProps { + count?: number; +} + +declare const RequiredFunctionComponent: FC; +declare const RequiredClassComponent: ComponentClass; +declare const RequiredMemoComponent: MemoExoticComponent>; +declare const RequiredForwardRefComponent: ForwardRefExoticComponent>; +declare const OptionalPropsComponent: FC; +declare const NoPropsComponent: FC; +declare const DismissableComponent: FC<{ dismiss: (data: string, role: string) => void }>; +declare const UntypedComponent: FC; +declare const overlayElement: JSX.Element; + +// None of these functions are invoked. They exist so `npm run typecheck` checks the +// calls inside them. +function componentPropsAreTypeChecked() { + useIonModalSignature(RequiredFunctionComponent, { title: 'Modal', count: 1 }); + useIonModalSignature(RequiredClassComponent, { title: 'Modal' }); + useIonModalSignature(RequiredMemoComponent, { title: 'Modal' }); + useIonModalSignature(RequiredForwardRefComponent, { title: 'Modal' }); + useIonPopoverSignature(RequiredFunctionComponent, { title: 'Popover', count: 1 }); + useIonPopoverSignature(RequiredClassComponent, { title: 'Popover' }); + + useIonPopoverSignature(RequiredMemoComponent, { title: 'Popover' }); + useIonPopoverSignature(RequiredForwardRefComponent, { title: 'Popover' }); + + // @ts-expect-error a required prop may not be omitted + useIonModalSignature(RequiredFunctionComponent, { count: 1 }); + // @ts-expect-error + useIonPopoverSignature(RequiredClassComponent, { count: 1 }); + + // @ts-expect-error unknown props are not accepted + useIonModalSignature(RequiredFunctionComponent, { title: 'Modal', unknown: true }); + // @ts-expect-error + useIonPopoverSignature(RequiredFunctionComponent, { title: 'Popover', unknown: true }); + + // @ts-expect-error props must match the declared types + useIonModalSignature(RequiredFunctionComponent, { title: 1 }); + // @ts-expect-error + useIonPopoverSignature(RequiredFunctionComponent, { title: 1 }); + + // @ts-expect-error a forwarded ref does not exempt a component from the check + useIonModalSignature(RequiredForwardRefComponent, { title: 1 }); +} + +// A rest-tuple signature still accepts an explicit type argument, so apps can pin the +// props type instead of relying on inference. +function explicitTypeArgumentsAreSupported() { + useIonModalSignature(RequiredFunctionComponent, { title: 'Modal' }); + useIonPopoverSignature(RequiredFunctionComponent, { title: 'Popover' }); + + // @ts-expect-error an explicit type argument still checks the props + useIonModalSignature(RequiredFunctionComponent, { count: 1 }); +} + +// Every call below omits `componentProps`, which `RequiredProps` does not allow. +function componentPropsAreRequiredWhenTheComponentRequiresThem() { + // @ts-expect-error + useIonModalSignature(RequiredFunctionComponent); + // @ts-expect-error + useIonModalSignature(RequiredClassComponent); + // @ts-expect-error + useIonModalSignature(RequiredMemoComponent); + // @ts-expect-error + useIonPopoverSignature(RequiredFunctionComponent); + // @ts-expect-error + useIonPopoverSignature(RequiredClassComponent); + // @ts-expect-error + useIonPopoverSignature(RequiredMemoComponent); +} + +// Untyped components keep the pre-v9 behavior, so upgrading apps don't get a new +// build error out of this. +function untypedComponentsStayPermissive() { + useIonModalSignature(UntypedComponent); + useIonModalSignature(UntypedComponent, { anything: true }); + useIonPopoverSignature(UntypedComponent); + useIonPopoverSignature(UntypedComponent, { anything: true }); +} + +function componentPropsAreOptionalWhenTheComponentHasNoRequiredProps() { + useIonModalSignature(NoPropsComponent); + useIonPopoverSignature(NoPropsComponent); + + useIonModalSignature(OptionalPropsComponent); + useIonModalSignature(OptionalPropsComponent, { count: 1 }); + useIonPopoverSignature(OptionalPropsComponent); + useIonPopoverSignature(OptionalPropsComponent, { count: 1 }); +} + +function jsxElementsRemainPermissive() { + useIonModalSignature(overlayElement); + useIonModalSignature(overlayElement, { anything: true }); + useIonPopoverSignature(overlayElement); + useIonPopoverSignature(overlayElement, { anything: true }); +} + +// Inline components need their props annotated, since `Props` is inferred from the +// component rather than from `componentProps`. See the `NoInfer` note in `useIonModal`. +function inlineComponentsAnnotateTheirProps() { + useIonModalSignature(({ name }: { name: string }) =>
Hello {name}.
, { name: 'Dave' }); + useIonPopoverSignature(({ name }: { name: string }) =>
Hello {name}.
, { name: 'Dave' }); +} + +// Overlays commonly pass `dismiss` back to the component through `componentProps`. +// That reads the binding the hook is still declaring, so it only compiles while +// `componentProps` stays out of inference. +function selfReferencingDismissCompiles() { + const [, dismissModal] = useIonModalSignature(DismissableComponent, { + dismiss: (data: string, role: string) => dismissModal(data, role), + }); + + const [, dismissPopover] = useIonPopoverSignature(DismissableComponent, { + dismiss: (data: string, role: string) => dismissPopover(data, role), + }); +} + +// Referenced so `noUnusedLocals` doesn't flag them. Keeping them unexported is what +// keeps the emitted declaration file empty. +void [ + componentPropsAreTypeChecked, + explicitTypeArgumentsAreSupported, + componentPropsAreRequiredWhenTheComponentRequiresThem, + untypedComponentsStayPermissive, + componentPropsAreOptionalWhenTheComponentHasNoRequiredProps, + jsxElementsRemainPermissive, + inlineComponentsAnnotateTheirProps, + selfReferencingDismissCompiles, +]; + +describe('overlay hook types', () => { + it('type checks component props at compile time', () => { + // The assertions in this file are enforced by `npm run typecheck`, which CI runs + // for this package. ts-jest sets `isolatedModules` in `tsconfig.spec.json` and so + // doesn't type check, which leaves nothing to assert at runtime. + }); +}); diff --git a/packages/react/src/hooks/useIonModal.ts b/packages/react/src/hooks/useIonModal.ts index aee3ea40e40..82e4b8523e6 100644 --- a/packages/react/src/hooks/useIonModal.ts +++ b/packages/react/src/hooks/useIonModal.ts @@ -1,6 +1,7 @@ import type { ModalOptions } from '@ionic/core/components'; import { modalController } from '@ionic/core/components'; import { defineCustomElement } from '@ionic/core/components/ion-modal.js'; +import type { ComponentType } from 'react'; import { useCallback } from 'react'; import type { ReactComponentOrElement } from '../models/ReactComponentOrElement'; @@ -10,12 +11,30 @@ import { useOverlay } from './useOverlay'; // TODO(FW-2959): types +// The `NoInfer` below keeps `componentProps` out of inference, so `Props` comes from +// the component alone. Overlays commonly pass `dismiss` back in through +// `componentProps`, and inferring from it would need the type of `dismiss` while that +// binding is still being declared, which TypeScript rejects as circular. The cost is +// that an inline component with no annotated props resolves to `{}`. + +/** + * A hook for presenting/dismissing an IonModal component + * @param component The component that the modal will show. Can be a React Component or a functional component + * @param componentProps The props that will be passed to the component. Required when the component declares required props + * @returns Returns the present and dismiss methods in an array + */ +export function useIonModal( + ...args: {} extends Props + ? [component: ComponentType, componentProps?: NoInfer] + : [component: ComponentType, componentProps: NoInfer] +): UseIonModalResult; /** * A hook for presenting/dismissing an IonModal component - * @param component The component that the modal will show. Can be a React Component, a functional component, or a JSX Element + * @param component A JSX Element that the modal will show. Props are already bound to the element, so `componentProps` is not type checked * @param componentProps The props that will be passed to the component, if required * @returns Returns the present and dismiss methods in an array */ +export function useIonModal(component: JSX.Element, componentProps?: any): UseIonModalResult; export function useIonModal(component: ReactComponentOrElement, componentProps?: any): UseIonModalResult { const controller = useOverlay( 'IonModal', diff --git a/packages/react/src/hooks/useIonPopover.ts b/packages/react/src/hooks/useIonPopover.ts index c63df7b26e0..eb6743f7334 100644 --- a/packages/react/src/hooks/useIonPopover.ts +++ b/packages/react/src/hooks/useIonPopover.ts @@ -1,6 +1,7 @@ import type { PopoverOptions } from '@ionic/core/components'; import { popoverController } from '@ionic/core/components'; import { defineCustomElement } from '@ionic/core/components/ion-popover.js'; +import type { ComponentType } from 'react'; import { useCallback } from 'react'; import type { ReactComponentOrElement } from '../models/ReactComponentOrElement'; @@ -10,12 +11,26 @@ import { useOverlay } from './useOverlay'; // TODO(FW-2959): types +// The `NoInfer` below does the same job as in `useIonModal`. See the note there. + +/** + * A hook for presenting/dismissing an IonPopover component + * @param component The component that the popover will show. Can be a React Component or a functional component + * @param componentProps The props that will be passed to the component. Required when the component declares required props + * @returns Returns the present and dismiss methods in an array + */ +export function useIonPopover( + ...args: {} extends Props + ? [component: ComponentType, componentProps?: NoInfer] + : [component: ComponentType, componentProps: NoInfer] +): UseIonPopoverResult; /** - * A hook for presenting/dismissing an IonPicker component - * @param component The component that the popover will show. Can be a React Component, a functional component, or a JSX Element + * A hook for presenting/dismissing an IonPopover component + * @param component A JSX Element that the popover will show. Props are already bound to the element, so `componentProps` is not type checked * @param componentProps The props that will be passed to the component, if required * @returns Returns the present and dismiss methods in an array */ +export function useIonPopover(component: JSX.Element, componentProps?: any): UseIonPopoverResult; export function useIonPopover(component: ReactComponentOrElement, componentProps?: any): UseIonPopoverResult { const controller = useOverlay( 'IonPopover', diff --git a/packages/react/test/base/src/pages/overlay-hooks/ModalHook.tsx b/packages/react/test/base/src/pages/overlay-hooks/ModalHook.tsx index fe3dfd0c284..f581d3e506a 100644 --- a/packages/react/test/base/src/pages/overlay-hooks/ModalHook.tsx +++ b/packages/react/test/base/src/pages/overlay-hooks/ModalHook.tsx @@ -51,7 +51,7 @@ const ModalHook: React.FC = () => { setCount(count + 1); }, [count, setCount]); - const handleDismissWithComponent = useCallback((data: any, role: string) => { + const handleDismissWithComponent = useCallback((data?: any, role?: string) => { dismissWithComponent(data, role); // eslint-disable-next-line react-hooks/exhaustive-deps }, []); @@ -87,6 +87,9 @@ const ModalHook: React.FC = () => { const [presentSecondaryModal] = useIonModal(ModalSecondary); const [presentRootModal, dismissRootModal] = useIonModal(Body, { + type: 'Root', + count: count, + onIncrement: handleIncrement, onDismiss: () => { dismissRootModal(); presentSecondaryModal();