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.

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

Check mounted state before using setState in onSubmit #1152

Closed
chrisngobanh opened this issue Jul 28, 2022 · 1 comment · Fixed by #1170
Closed

Check mounted state before using setState in onSubmit #1152

chrisngobanh opened this issue Jul 28, 2022 · 1 comment · Fixed by #1170
Assignees
Labels
Area: Core Affects the uniforms package Type: Bug Bug reports and their fixes
Milestone

Comments

@chrisngobanh
Copy link

chrisngobanh commented Jul 28, 2022

  • Version: Latest

onSubmit(event?: SyntheticEvent) {
if (event) {
event.preventDefault();
event.stopPropagation();
}
this.setState(state => (state.submitted ? null : { submitted: true }));
const result = this.props.onSubmit(this.getModel('submit'));
if (!(result instanceof Promise)) {
return Promise.resolve();
}
this.setState({ submitting: true });
return result.finally(() => {
this.setState({ submitting: false });
});
}

In BaseForm's onSubmit method, there may be instances where calling props.onSubmit would cause the component to be unmounted (e.g. if an API call is successful, navigate to a different page; On failure, error boundary catches it and shows an error page). In this scenario, a Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. error would be thrown when the setState is called.

A quick fix to this issue would be to check this.mounted before calling setState. Another fix would be to bring back the onSubmitSuccess and onSubmitFailure props to be called after all setStates and instruct the consumer to put their logic in here that would result in the form component being unmounted

@radekmie radekmie self-assigned this Aug 9, 2022
@radekmie radekmie added Type: Bug Bug reports and their fixes Area: Core Affects the uniforms package labels Aug 9, 2022
@radekmie radekmie added this to Needs triage in Open Source (migrated) via automation Aug 9, 2022
@radekmie radekmie added this to the v3.10 milestone Aug 9, 2022
@radekmie
Copy link
Contributor

radekmie commented Aug 9, 2022

Hey @chrisngobanh, and thanks for the report! Agreed, we should add a check for the component being mounted there (we already have it, so it's going to be easy). Maybe you'd like to file a PR for that?

Also, heads up: facebook/react#22114.

@radekmie radekmie moved this from Needs triage to To do in Open Source (migrated) Aug 19, 2022
@radekmie radekmie moved this from To do to Review in Open Source (migrated) Oct 7, 2022
Open Source (migrated) automation moved this from Review to Closed Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Core Affects the uniforms package Type: Bug Bug reports and their fixes
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants