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 controlled radios, maybe for real this time #27443

Merged
merged 2 commits into from
Oct 2, 2023
Merged

Fix controlled radios, maybe for real this time #27443

merged 2 commits into from
Oct 2, 2023

Conversation

sophiebits
Copy link
Collaborator

Fixes #26876 for real?

In 18.2.0 (last stable), we set .checked unconditionally:

https://github.com/facebook/react/blob/v18.2.0/packages/react-dom/src/client/ReactDOMInput.js#L129-L135

This is important because if we are updating two radios' checkedness from (false, true) to (true, false), we need to make sure that input2.checked is explicitly set to false, even though setting input1.checked = true already unchecks input2.

I think this fix is not complete because there is no guarantee that all the inputs rerender at the same time? Hence the TODO. But in practice they usually would and I think this is comparable to what we had before.

Also treating function and symbol as false like we used to and like we do on initial mount.

Fixes #26876 for real?

In 18.2.0 (last stable), we set .checked unconditionally:

https://github.com/facebook/react/blob/v18.2.0/packages/react-dom/src/client/ReactDOMInput.js#L129-L135

This is important because if we are updating two radios' checkedness from (false, true) to (true, false), we need to make sure that input2.checked is explicitly set to false, even though setting `input1.checked = true` already unchecks input2.

I think this fix is not complete because there is no guarantee that all the inputs rerender at the same time? Hence the TODO. But in practice they usually would and I _think_ this is comparable to what we had before.

Also treating function and symbol as false like we used to and like we do on initial mount.
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Sep 30, 2023
@react-sizebot
Copy link

react-sizebot commented Sep 30, 2023

Comparing: a6ed60a...6022257

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js +0.02% 167.55 kB 167.58 kB = 52.14 kB 52.14 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js +0.02% 176.21 kB 176.24 kB = 54.84 kB 54.84 kB
facebook-www/ReactDOM-prod.classic.js = 564.39 kB 564.42 kB = 99.37 kB 99.37 kB
facebook-www/ReactDOM-prod.modern.js = 548.11 kB 548.14 kB = 96.45 kB 96.45 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 6022257

@sophiebits
Copy link
Collaborator Author

Verified this fixes #27394 (comment). I hate radio buttons.

@sophiebits sophiebits merged commit 4f4c52a into main Oct 2, 2023
36 checks passed
github-actions bot pushed a commit that referenced this pull request Oct 2, 2023
Fixes #26876 for real?

In 18.2.0 (last stable), we set .checked unconditionally:

https://github.com/facebook/react/blob/v18.2.0/packages/react-dom/src/client/ReactDOMInput.js#L129-L135

This is important because if we are updating two radios' checkedness
from (false, true) to (true, false), we need to make sure that
input2.checked is explicitly set to false, even though setting
`input1.checked = true` already unchecks input2.

I think this fix is not complete because there is no guarantee that all
the inputs rerender at the same time? Hence the TODO. But in practice
they usually would and I _think_ this is comparable to what we had
before.

Also treating function and symbol as false like we used to and like we
do on initial mount.

DiffTrain build for [4f4c52a](4f4c52a)
alunyov pushed a commit to alunyov/react that referenced this pull request Oct 11, 2023
Fixes facebook#26876 for real?

In 18.2.0 (last stable), we set .checked unconditionally:


https://github.com/facebook/react/blob/v18.2.0/packages/react-dom/src/client/ReactDOMInput.js#L129-L135

This is important because if we are updating two radios' checkedness
from (false, true) to (true, false), we need to make sure that
input2.checked is explicitly set to false, even though setting
`input1.checked = true` already unchecks input2.

I think this fix is not complete because there is no guarantee that all
the inputs rerender at the same time? Hence the TODO. But in practice
they usually would and I _think_ this is comparable to what we had
before.

Also treating function and symbol as false like we used to and like we
do on initial mount.
ztanner added a commit to vercel/next.js that referenced this pull request Oct 16, 2023
…experimental prefix for server action APIs (#56809)

The latest React canary builds have a few changes that need to be
adopted for compatability.

1. the `useFormState` and `useFormStatus` hooks in `react-dom` and the
`formData` opiont in `react-dom/server` are no longer prefixed with
`experimental_`
2. server content (an undocumented React feature) has been removed. Next
only had trivial intenral use of this API and did not expose a coherent
feature to Next users (no ability to seed context on refetches). It is
still possible that some users used the React server context APIs which
is why this should go into Next 14.

### React upstream changes

- facebook/react#27513
- facebook/react#27514
- facebook/react#27511
- facebook/react#27508
- facebook/react#27502
- facebook/react#27474
- facebook/react#26789
- facebook/react#27500
- facebook/react#27488
- facebook/react#27458
- facebook/react#27471
- facebook/react#27470
- facebook/react#27464
- facebook/react#27456
- facebook/react#27462
- facebook/react#27461
- facebook/react#27460
- facebook/react#27459
- facebook/react#27454
- facebook/react#27457
- facebook/react#27453
- facebook/react#27401
- facebook/react#27443
- facebook/react#27445
- facebook/react#27364
- facebook/react#27440
- facebook/react#27436

---------

Co-authored-by: Zack Tanner <zacktanner@gmail.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: Jiachi Liu <inbox@huozhi.im>
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
Fixes facebook#26876 for real?

In 18.2.0 (last stable), we set .checked unconditionally:


https://github.com/facebook/react/blob/v18.2.0/packages/react-dom/src/client/ReactDOMInput.js#L129-L135

This is important because if we are updating two radios' checkedness
from (false, true) to (true, false), we need to make sure that
input2.checked is explicitly set to false, even though setting
`input1.checked = true` already unchecks input2.

I think this fix is not complete because there is no guarantee that all
the inputs rerender at the same time? Hence the TODO. But in practice
they usually would and I _think_ this is comparable to what we had
before.

Also treating function and symbol as false like we used to and like we
do on initial mount.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
Fixes #26876 for real?

In 18.2.0 (last stable), we set .checked unconditionally:

https://github.com/facebook/react/blob/v18.2.0/packages/react-dom/src/client/ReactDOMInput.js#L129-L135

This is important because if we are updating two radios' checkedness
from (false, true) to (true, false), we need to make sure that
input2.checked is explicitly set to false, even though setting
`input1.checked = true` already unchecks input2.

I think this fix is not complete because there is no guarantee that all
the inputs rerender at the same time? Hence the TODO. But in practice
they usually would and I _think_ this is comparable to what we had
before.

Also treating function and symbol as false like we used to and like we
do on initial mount.

DiffTrain build for commit 4f4c52a.
@BadrElfarri BadrElfarri mentioned this pull request Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Radio button onChange not called in current React Canary
5 participants