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

fix: submit event bubbling with React 17 #2105

Closed
wants to merge 1 commit into from

Conversation

dreadheadsic
Copy link

@dreadheadsic dreadheadsic commented Oct 24, 2020

Reasons for making this change

React 17 has changed the way event handlers are attached - more info and because of that, submit event doesn't bubble up unless you explicitly tell it to bubble.
This issue affects submitting the form programmatically.

fixes #2104

@dreadheadsic dreadheadsic changed the title fix: submit event bubbling fix: submit event bubbling with React 17 Oct 24, 2020
Copy link
Member

@epicfaace epicfaace left a comment

Choose a reason for hiding this comment

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

@dreadheadsic apologies for the delay. I'm having trouble understanding why the changes in the way react 17 attaches event listeners makes the onSubmit event no longer bubble, as well as why onSubmit not bubbling prevents users from submitting the form programmatically. Could you explain?

I'm also wondering if we can add a test to ensure we catch this issue with React 17.

@epicfaace
Copy link
Member

Specifically, I don't understand why we need to bubble the onSubmit event beyond this.formElement, when React 17 attaches form listeners on the element directly (this.formElement) anyway.

@dreadheadsic
Copy link
Author

@epicfaace no worries. You know what, when i gave it a second thought, it didn't made sense to me either, why sudden need to bubble the event up, when react just changed where it attaches handlers (used to be on document, now it does on root node). So i went through that react blog post and figured, this change is targeted towards running two different react versions - Thanks to this change, it is now safer to embed a React tree managed by one version inside a tree managed by a different React version.
So what was the issue in my case? I've bumped react to 17.0 in my package, but @rjsf/core still depends on v16, so i had two react versions running. I've tested this locally by bumping react dep in @rjsf/core to v17 and voila, everything worked without changes in bubbling.
To conclude, @rjsf works fine with react 17 and this PR is irrelevant, you can throw it away :)

@epicfaace
Copy link
Member

Great, thanks for looking into this!

@robertedjones
Copy link

I have forked the repo and changed the peer dependency to 17 but the issue is not fixed for me. This PR however does fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

React 17 submit event not bubbling
3 participants