[DateTimePicker] Publish sistent's own props type instead of the optional peer's - #1754
Open
Valyrian-Code wants to merge 1 commit into
Open
Conversation
…al peer's
`dist/index.d.ts` named `@mui/x-date-pickers` in two places: the barrel's
`export type { DateTimePickerProps }` re-export, and the exported component's
own `ForwardRefExoticComponent<DateTimePickerProps & ...>` declaration. The peer
is optional, so a consumer is entitled to skip it, and the reference then fails
two ways: `TS2307` under `skipLibCheck: false`, and a silent `any` under
`skipLibCheck: true`.
Removing only the re-export leaves the second reference, which keeps the import
alive on its own, so declare sistent's own `DateTimePickerProps` instead. Named
props stay checked; a string index signature keeps every other picker prop
accepted and forwarded, so this loosens rather than narrows the public surface.
`forwardRef`'s result is annotated explicitly because `PropsWithoutRef<P>`
resolves to `Omit<P, 'ref'>` for a `P` with an index signature, which collapses
the interface to `{ [x: string]: unknown }` and silently discards every named
prop's type.
With the peer absent, the issue's own `IsAny<DateTimePickerProps>` probe now
fails to compile, which is the fix: the type is real rather than `any`. Both
declaration bundles reference the peer zero times, so the
`@mui/x-date-pickers` entry in `OPTIONAL_PEERS_ON_THE_RECORD` is deleted -- the
guard asserts each exemption is still needed and would fail on a stale one.
Signed-off-by: Rajveer Bishnoi <260926695+Valyrian-Code@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughDateTimePicker now exports local props typings instead of exposing ChangesOptional peer type-surface cleanup
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Notes for Reviewers
This PR fixes #1749
@mui/x-date-pickersis an optional peer, so a consumer is entitled to skip it, but the published declaration bundle named it and the reference then failed two ways:TS2307underskipLibCheck: false, and a silentanyunderskipLibCheck: true.Removing the re-export alone was not enough
The issue suggests dropping the barrel's
export type { DateTimePickerProps }. I tried exactly that first and rebuilt —dist/index.d.tsstill had:because the exported component was itself declared with that type:
so the import survived to serve that declaration. Two references, not one — it went 2 → 1, which means the
@mui/x-date-pickersentry inOPTIONAL_PEERS_ON_THE_RECORDstill could not be deleted (the guard asserts each exemption is still needed and fails on a stale one).What this does instead
DateTimePickernow publishes sistent's ownDateTimePickerPropsinterface rather than aliasing the peer's. Named props stay checked; a string index signature keeps every other picker prop accepted and forwarded, so this loosens rather than narrows the public surface. MUI's real type stays as a module-localimport typeused only in positions the declaration bundle erases, so the runtimeReact.lazydeferral is untouched.One non-obvious detail, flagged because it is easy to "simplify" away in review: the
forwardRefresults are annotated explicitly with aDateTimePickerComponentalias. Left to inference,forwardRef<T, P>returnsForwardRefExoticComponent<PropsWithoutRef<P> & ...>, andPropsWithoutRef<P>resolves toOmit<P, 'ref'>whenPhas an index signature — whosekeyof Pisstring | number, collapsing the interface to{ [x: string]: unknown }and silently discarding every named prop's type.<DateTimePicker value="not a date" />then compiles clean, which would be worse than the bug being fixed. The built output isdeclare const DateTimePicker: DateTimePickerComponent;, not the collapsedOmit<..., "ref">form.I also reworded a comment in
DateTimePicker.tsxbecause my first draft quoted a literalimport ... from '@mui/x-date-pickers/...'statement in prose, whichoptionalPeerDependencies.test.tscorrectly flagged — that scanner deliberately does not strip comments. Reworded the prose rather than touching the guard.Verification
Against a clean consumer built from
npm pack, with both optional peers confirmed absent:dist/index.d.tsanddist/index.d.mtsreference@mui/x-date-pickerszero times (specifier positions, comments stripped, same detector the guard uses).any:require('@sistent/sistent')still loads with the peers absent, so the runtime half is unaffected.skipLibCheck: falsethe only remainingTS2307s come from@meshery/schemasreferencing@reduxjs/toolkit— the pre-existingUNDECLARED_BY_DESIGNexemption, unrelated to this change.DateTimePickerPropsvalue into the component still compiles, as do two-parameteronChangehandlers,views/ampm/slotProps/sx/ref/data-testid, whilevalue={'not a date'}andonChange={(n: number) => {}}still error.CI=1 jest: 466 passing, 26 suites.eslint .clean.prettier --checkclean on the touched files.Because the count is now zero, the
@mui/x-date-pickersentry is deleted fromOPTIONAL_PEERS_ON_THE_RECORDand the surrounding comment updated —reactis the sole remaining entry and it is permanent rather than a deferred fix, which the old wording ("the fix is to stop naming them in the public type surface") no longer described.AGENTS.mdgets the same correction plus a short note that dropping a type re-export does not on its own remove a peer an exported component is still typed with.One known limitation
Assigning a MUI props value to sistent's
DateTimePickerPropsin a non-JSX position stops compiling:An interface source gets no implicit index signature. JSX spread — the common case — works. This cannot be fixed without dropping the index signature, which would break the far more common spread and MUI-only-prop cases, so I took this trade deliberately.
Happy to switch to the smaller variant instead — dropping the public
DateTimePickerPropsexport entirely and keeping the props interface non-exported — which matches the issue's literal wording, also reaches zero, and avoids the index-signature loosening. I raised both options on the issue; this is the one that keeps the existing public type working, but it is your call and the change is small either way.Signed commits
Summary by CodeRabbit
Bug Fixes
DateTimePicker.Documentation