Skip to content

@types/redux-form Removed Partial from onSubmit values#24816

Merged
mhegazy merged 1 commit intoDefinitelyTyped:masterfrom
Rockson:master
Apr 14, 2018
Merged

@types/redux-form Removed Partial from onSubmit values#24816
mhegazy merged 1 commit intoDefinitelyTyped:masterfrom
Rockson:master

Conversation

@Rockson
Copy link
Copy Markdown
Contributor

@Rockson Rockson commented Apr 8, 2018

see iss: #24376

Please fill in this template.

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Test the change in your own code. (Compile and run.)
  • Add or edit tests to reflect the change. (Run with npm test.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

@Rockson
Copy link
Copy Markdown
Contributor Author

Rockson commented Apr 8, 2018

Actually I think initialValues can be a Partial because you should be able to initialize a form with partial data but the onSubmit data shouldn't be a partial because if a field is required u check it in the form with validators so u will reach onSubmit with exactly FormData not Partial

That's why I've changed only the onSubmit typedefs

@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Apr 8, 2018

@Rockson Thank you for submitting this PR!

🔔 @CarsonF @aikoven @LKay @bancek @alsiola @tehbi4 @huwmartin - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.


export type FormSubmitHandler<FormData = {}, P = {}> =
(values: Partial<FormData>, dispatch: Dispatch<any>, props: P) => void | FormErrors<FormData> | Promise<any>;
(values: FormData, dispatch: Dispatch<any>, props: P) => void | FormErrors<FormData> | Promise<any>;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was rather confused when I looked at this because FormData is an actual DOM API constructor/instance and it makes no sense to have a Partial<FormData>. Naming it TFormData might make it less confusing, but that's not the point of the PR 🤷‍♀️

@typescript-bot typescript-bot added the Unmerged The author did not merge the PR when it was ready. label Apr 13, 2018
@typescript-bot
Copy link
Copy Markdown
Contributor

After 5 days, no one has reviewed the PR 😞. A maintainer will be reviewing the PR in the next few days and will either merge it or request revisions. Thank you for your patience!

@mhegazy mhegazy merged commit dda14df into DefinitelyTyped:master Apr 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Unmerged The author did not merge the PR when it was ready.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants