fix(types): allow design argument in DateTimePickerAndroid.dismiss - #1059
Open
giaBaoJS wants to merge 1 commit into
Open
fix(types): allow design argument in DateTimePickerAndroid.dismiss#1059giaBaoJS wants to merge 1 commit into
giaBaoJS wants to merge 1 commit into
Conversation
The runtime signature is dismiss(mode, design) since the Material 3 pickers were added in react-native-datetimepicker#952, and the library itself calls it with two arguments in src/datetimepicker.android.js. The public TypeScript declaration still only accepts one, so TypeScript users cannot dismiss a Material picker without casting.
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.
Summary
DateTimePickerAndroid.dismissaccepts a seconddesignargument at runtime, but the public TypeScript declaration only declares one parameter, so TypeScript users cannot dismiss a Material 3 picker without a cast.Runtime (
src/DateTimePickerAndroid.android.js:143):Declaration before this PR (
src/index.d.ts:266):The library itself already calls the two-argument form in
src/datetimepicker.android.js:47:The
designparameter was added to the runtime in #952 (Material 3 pickers) butindex.d.tswas never updated, sodismisshas been under-typed since 9.0.0. Without it, a TypeScript app that opens a Material picker imperatively and then wants to dismiss it either has to omitdesign(which silently falls back to thedefaultpickers and dismisses the wrong dialog) or write(DateTimePickerAndroid.dismiss as any)(mode, 'material').This is a declaration-only change. No runtime behaviour changes.
I also updated the signature shown in the
Android imperative apisection ofREADME.mdso the documented API matches.Test Plan
The repository has no TypeScript type-check step, so I verified the declaration with a local
tscrun against the repo's owntypescript@5.8.3and a scratch file (not committed):Before the fix, with
masterata523007:After the fix:
Counterfactual, reverting only
src/index.d.tsback to theHEADversion and restoring it:The new parameter is narrowly typed rather than
any, so invalid values are still rejected:Repository checks:
yarn flowcould not run on this machine (the bundledflow-binbinary is x86-only and fails to spawn on arm64). No Flow file is touched by this PR; the Flow annotations ondismissalready carry thedesignparameter.Note on CI:
e2e_ios (old arch),e2e_ios (new arch)ande2e_android (old arch)are already failing onmasterat the base commita523007(run 33481502431), whileanalyse_jspasses. Those failures are pre-existing and unrelated to this change.What's required for testing (prerequisites)?
yarn install, plus the repo's owntypescriptdevDependency for the type check above.What are the steps to reproduce (after prerequisites)?
In a TypeScript file, call
DateTimePickerAndroid.dismiss('date', 'material'). Onmasterit fails to compile withTS2554: Expected 1 arguments, but got 2, even though that is exactly what the library does internally.Compatibility
Checklist
README.mdexample/App.js)