-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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.
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
[core] fix(InputGroup): async controlled updates #4266
Conversation
[core] fix(InputGroup): async controlled updatesPreviews: documentation | landing | table |
@adidahiya I've tested this PR against the product where we originally found the issue, and it does appear to fix it. However, unless I'm mistaken, we're still relying on the state update here being synchronous. In practice, given how small and simple this component is, it seems likely this state update will always be synchronous (hence why it appears to fix the issue), but I don't think that's guaranteed by React at all. I'm not sure if there's a better fix though, and maybe we're ok with it just being significantly less vulnerable to the issue than before? |
fix: pass down inputRef to inputPreviews: documentation | landing | table |
I tried fixing the tests but I don't see an obvious reason why they would fail only in React 15. I will come back to this later this week, but if you have any ideas I would welcome them in the meantime |
🤦 I had forgot to apply the react-lifecycles-compat polyfill to make |
apply react-lifecycles-compat polyfillPreviews: documentation | landing | table |
Alternative fix for issue described in #4262 and facebook/react#3926
Checklist
Changes proposed in this pull request:
Add a new private component,
AsyncControllableInput
, which works around the problems in the linked React issue, alleviating some bugs we've experienced with IMEReviewers should focus on:
Test this in a real product and make sure it doesn't break anything
Screenshot
N/A