Skip to content
Open
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
11 changes: 9 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 '<peer>'` 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
Expand Down
18 changes: 8 additions & 10 deletions src/__testing__/publishedTypeSurfaceDependencies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
74 changes: 66 additions & 8 deletions src/base/DateTimePicker/DateTimePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<T, P>` inference.
* That helper returns `ForwardRefExoticComponent<PropsWithoutRef<P> & ...>`, and
* for a `P` carrying a string index signature `PropsWithoutRef` resolves to
* `Omit<P, 'ref'>`, whose `keyof P` is `string | number` - collapsing the whole
* interface to `{ [x: string]: unknown }` and discarding every named prop's type.
* `<DateTimePicker value="not a date" />` would then compile silently, which is
* the same unchecked-prop failure this file exists to avoid.
*/
type DateTimePickerComponent = React.ForwardRefExoticComponent<
DateTimePickerProps & React.RefAttributes<HTMLDivElement>
>;

const LazyDateTimePicker = React.lazy(async () => {
const [{ AdapterDateFns }, { LocalizationProvider }, { DateTimePicker: MuiDateTimePicker }] =
await Promise.all([
Expand All @@ -36,19 +90,23 @@ const LazyDateTimePicker = React.lazy(async () => {
);
});

const ResolvedDateTimePicker = React.forwardRef<HTMLDivElement, MuiDateTimePickerProps>(
(props, ref) => (
<LocalizationProvider dateAdapter={AdapterDateFns}>
<MuiDateTimePicker {...props} ref={ref} />
</LocalizationProvider>
)
);
const ResolvedDateTimePicker: DateTimePickerComponent = React.forwardRef<
HTMLDivElement,
DateTimePickerProps
>((props, ref) => (
<LocalizationProvider dateAdapter={AdapterDateFns}>
<MuiDateTimePicker {...(props as MuiDateTimePickerProps)} ref={ref} />
</LocalizationProvider>
));
ResolvedDateTimePicker.displayName = 'ResolvedDateTimePicker';

return { default: ResolvedDateTimePicker };
});

const DateTimePicker = React.forwardRef<HTMLDivElement, MuiDateTimePickerProps>((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.
<React.Suspense fallback={null}>
Expand Down
3 changes: 1 addition & 2 deletions src/base/DateTimePicker/index.tsx
Original file line number Diff line number Diff line change
@@ -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 };
Loading