Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

react-native if onChange callback is async ConnectedField won't trigger the change action #4391

Closed
TomaszChmielPagepro opened this issue Mar 1, 2019 · 2 comments · Fixed by #4596
Closed

react-native if onChange callback is async ConnectedField won't trigger the change action #4391

TomaszChmielPagepro opened this issue Mar 1, 2019 · 2 comments · Fixed by #4596

Comments

@TomaszChmielPagepro
Copy link

Are you submitting a bug report or a feature request?

bug report

What is the current behavior?

If onChange prop is async function ConnectedField does not trigger the change action. I belive this is because async function always returns a Promise instance so this condition is always true
https://github.com/erikras/redux-form/blob/84359b3fb5be9f62e030657100ea1ddede4c5ef7/src/ConnectedField.js#L132

What is the expected behavior?

Maybe we should check for Promises here and wait resolve them and then use their result as value for defaultPrevented variable?

Sandbox Link

https://codesandbox.io/s/l7vp6w6nyz

File: App.js
Line: 16

What's your environment?

react-native, both iOS and Android
TomaszChmielPagepro added a commit to Pagepro/redux-form that referenced this issue Mar 2, 2019
@TomaszChmielPagepro
Fix react-native async onChange prop in ConnectedField redux-form#4391
e102d73
@TomaszChmielPagepro
Copy link
Author

Looks like test need some fixes now. I will fix them as sson as possible. Just tell me if you think this is a good solution. Thanks!

TomaszChmielPagepro added a commit to Pagepro/redux-form that referenced this issue Mar 9, 2019
@TomaszChmielPagepro
fixed tests for redux-form#4391
ff5dcd0
@iamandrewluca
Copy link
Member

This it seems more as a feature request.
As I see in code and docs, onChange does not support async functions.
Returning a truthy value from onChange in ReactNative works like event.preventDefault()

timhwang21 added a commit to timhwang21/redux-form that referenced this issue Jan 10, 2020
@timhwang21
Fix onChange return affecting preventDefault outside RN
749da2e 
Issue redux-form#4309 adds a way to cancel dispatches from RN. However, the method
added also impacts React Web. This leads to a regression in React Web
due to application of RN behavior.

The current change locks the change from redux-form#4309 to RN, and maintains
existing behavior otherwise.

Related:
- redux-form#4391 outlines a case where this regression may cause problems. While
  the issue mentions RN, this can easily occur outside RN as well.
- redux-form#4401 describes a similar issue and a user-end fix.
@timhwang21 timhwang21 mentioned this issue Jan 10, 2020
Merged
iamandrewluca pushed a commit that referenced this issue Jan 13, 2020
@timhwang21 @iamandrewluca
fix(field): use onChange return as preventDefault only in RN (#4596)
12ad4ec 
Issue #4309 adds a way to cancel dispatches from RN. However, the method
added also impacts React Web. This leads to a regression in React Web
due to application of RN behavior.

The current change locks the change from #4309 to RN, and maintains
existing behavior otherwise.

Related:
- #4391 outlines a case where this regression may cause problems. While
  the issue mentions RN, this can easily occur outside RN as well.
- #4401 describes a similar issue and a user-end fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(field): use onChange return as preventDefault only in RN timhwang21/redux-form
2 participants
@iamandrewluca @TomaszChmielPagepro