diff --git a/AGENTS.md b/AGENTS.md index 63619d5b7..17a90235c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -94,10 +94,17 @@ is the guard. It reads the built `dist/index.d.ts` (CI's `node-checks.yml` runs when `CI` is set). It is also the source of truth for the two exemption lists and the per-package rationale behind each: packages that leak _undeclared_ (today the redux-facing surface reached through `src/actors/*` and `src/redux-persist/*`, whose remedy is its own opt-in entry point, not a -dependency), and packages that _are_ declared but only as optional peers, whose remedy is to stop -naming them in the public type surface. Every entry is asserted to still be needed, so it cannot +dependency), and packages that _are_ declared but only as optional peers, where the remedy is +usually to stop naming them in the public type surface - though the one remaining entry, `react`, +is permanent rather than deferred. Every entry is asserted to still be needed, so it cannot outlive its problem - read that file, not a copy here, before touching either list. +Note what "stop naming them" has to cover. Dropping a `export type { X } from ''` re-export is +not enough on its own if an _exported component_ is still typed with the peer's props: the +declaration bundle keeps the import alive to serve that declaration. #1749 was exactly this - the +props re-export and `DateTimePicker`'s own `ForwardRefExoticComponent<...>` type were two separate +references, and only removing both got the count to zero. + Declaring one has a downstream consequence a `devDependency` did not: a real `dependency` takes part in the consumer's own dedupe, so installing sistent can lift the consumer's copy of that package to satisfy sistent's range. `.github/workflows/test-meshery-integration.yml` pins a Meshery commit for diff --git a/src/__testing__/publishedTypeSurfaceDependencies.test.ts b/src/__testing__/publishedTypeSurfaceDependencies.test.ts index 5191646fd..e484dbb15 100644 --- a/src/__testing__/publishedTypeSurfaceDependencies.test.ts +++ b/src/__testing__/publishedTypeSurfaceDependencies.test.ts @@ -61,20 +61,18 @@ const UNDECLARED_BY_DESIGN = ['@reduxjs/toolkit', 'redux']; * * Kept separate from `UNDECLARED_BY_DESIGN` because the remedy differs: those * are missing from `package.json` entirely, these are present and merely - * optional, and the fix is to stop naming them in the public type surface. + * optional. Where an optional peer is named only incidentally the remedy is to + * stop naming it in the public type surface - that is what closed the + * `@mui/x-date-pickers` entry that used to sit here (#1749): `DateTimePicker` + * now publishes sistent's own props interface instead of aliasing the peer's. * - * - `@mui/x-date-pickers`: a live defect, not a design choice. The barrel does - * `export { DateTimePickerProps } from '@mui/x-date-pickers/DateTimePicker'`, - * so a consumer who skips the optional peer silently gets `any` for it. The - * real fix is to stop re-exporting that props type from the barrel, tracked in - * https://github.com/layer5io/sistent/issues/1749 - exempted here on the - * record so this guard can ship without also making a public-API change. * - `react`: optional in `package.json`, but every component's props type names * it and always will, and a consumer of a React component library has React. - * Same rationale as the `react` / `react-dom` exemption in the sibling - * `optionalPeerDependencies.test.ts`. + * So unlike the entry above this one has no remedy to reach for - it is a + * permanent exemption, not a deferred fix. Same rationale as the `react` / + * `react-dom` exemption in the sibling `optionalPeerDependencies.test.ts`. */ -const OPTIONAL_PEERS_ON_THE_RECORD = ['@mui/x-date-pickers', 'react']; +const OPTIONAL_PEERS_ON_THE_RECORD = ['react']; /** * JSDoc in the bundle carries fenced `@example` blocks with real-looking import diff --git a/src/base/DateTimePicker/DateTimePicker.tsx b/src/base/DateTimePicker/DateTimePicker.tsx index e7d461731..1287f8098 100644 --- a/src/base/DateTimePicker/DateTimePicker.tsx +++ b/src/base/DateTimePicker/DateTimePicker.tsx @@ -17,6 +17,60 @@ import React from 'react'; */ const OPTIONAL_PEERS = '@mui/x-date-pickers and date-fns'; +/** + * sistent's own props contract, deliberately not an alias of the peer's + * `DateTimePickerProps`. + * + * The runtime import above is deferred so the optional peer stays optional, but a + * type has no lazy form. Typing the exported component with MUI's props left the + * declaration bundle importing the peer's `DateTimePickerProps`, which a consumer + * who skipped the optional peer cannot resolve: `TS2307` under + * `skipLibCheck: false`, and a silent `any` under `skipLibCheck: true`. See + * `src/__testing__/publishedTypeSurfaceDependencies.test.ts`. + * + * Note that dropping the barrel's type re-export was not sufficient on its own - + * the exported component's own declaration was a second reference to the peer, + * and kept the import alive by itself. + * + * The named props are the ones sistent checks. The index signature keeps every + * other picker prop accepted and forwarded, so the pass-through is described + * without naming a package the consumer may not have. + */ +export interface DateTimePickerProps { + label?: React.ReactNode; + value?: Date | null; + defaultValue?: Date | null; + /** + * Trailing `...rest: never[]` rather than a named second parameter: MUI passes a + * validation context as the second argument, and spelling it here would make a + * handler that ignores it - which every call site in this repo does - the only + * assignable shape, rejecting the two-parameter handlers MUI's own type allows. + */ + onChange?: (value: Date | null, ...rest: never[]) => void; + disabled?: boolean; + readOnly?: boolean; + minDate?: Date; + maxDate?: Date; + minDateTime?: Date; + maxDateTime?: Date; + format?: string; + className?: string; + [prop: string]: unknown; +} + +/** + * Annotated explicitly rather than left to `React.forwardRef` inference. + * That helper returns `ForwardRefExoticComponent & ...>`, and + * for a `P` carrying a string index signature `PropsWithoutRef` resolves to + * `Omit`, whose `keyof P` is `string | number` - collapsing the whole + * interface to `{ [x: string]: unknown }` and discarding every named prop's type. + * `` would then compile silently, which is + * the same unchecked-prop failure this file exists to avoid. + */ +type DateTimePickerComponent = React.ForwardRefExoticComponent< + DateTimePickerProps & React.RefAttributes +>; + const LazyDateTimePicker = React.lazy(async () => { const [{ AdapterDateFns }, { LocalizationProvider }, { DateTimePicker: MuiDateTimePicker }] = await Promise.all([ @@ -36,19 +90,23 @@ const LazyDateTimePicker = React.lazy(async () => { ); }); - const ResolvedDateTimePicker = React.forwardRef( - (props, ref) => ( - - - - ) - ); + const ResolvedDateTimePicker: DateTimePickerComponent = React.forwardRef< + HTMLDivElement, + DateTimePickerProps + >((props, ref) => ( + + + + )); ResolvedDateTimePicker.displayName = 'ResolvedDateTimePicker'; return { default: ResolvedDateTimePicker }; }); -const DateTimePicker = React.forwardRef((props, ref) => ( +const DateTimePicker: DateTimePickerComponent = React.forwardRef< + HTMLDivElement, + DateTimePickerProps +>((props, ref) => ( // `null` rather than a spinner: the chunk resolves in a frame or two and a // flashing placeholder inside a form field reads as a bug. diff --git a/src/base/DateTimePicker/index.tsx b/src/base/DateTimePicker/index.tsx index 4fcb31ef9..100726753 100644 --- a/src/base/DateTimePicker/index.tsx +++ b/src/base/DateTimePicker/index.tsx @@ -1,5 +1,4 @@ -import type { DateTimePickerProps } from '@mui/x-date-pickers/DateTimePicker'; -import DateTimePicker from './DateTimePicker'; +import DateTimePicker, { type DateTimePickerProps } from './DateTimePicker'; export { DateTimePicker }; export type { DateTimePickerProps };