Skip to content

Commit

Permalink
Update tracked value after resetting radio group (#27394)
Browse files Browse the repository at this point in the history
Fixes #26876, I think. Review each commit separately (all assertions
pass in main already, except the last assertInputTrackingIsClean in
"should control radio buttons").

I'm actually a little confused on two things here:
* All the isCheckedDirty assertions are true. But I don't think we set
.checked unconditionally? So how does this happen?
* facebook/react#26876 (comment)
claims that
facebook/react@d962f35...1f248bd contains
the faulty change, but it doesn't appear to change the restoration logic
that I've touched here. (One difference outside restoration is that
updateProperties did previously set `.checked` when `nextProp !==
lastProp` whereas the new logic in updateInput is to set it when
`node.checked !== !!checked`.)

But it seems to me like we need this call here anyway, and if it fixes
it then it fixes it? I think technically speaking we probably should do
all the updateInput() calls and then all the updateValueIfChanged()
calls—in particular I think if clicking A changed the checked radio
button from B to C then the code as I have it would be incorrect, but
that also seems unlikely so idk whether to care.

cc @zhengjitf @Luk-z who did some investigation on the original issue

DiffTrain build for [3c27178a2f2c74f14d90613028e3929e1f06d830](facebook/react@3c27178)
  • Loading branch information
jerrydev0927 committed Sep 21, 2023
1 parent 17e3d0e commit 0aa4101
Show file tree
Hide file tree
Showing 15 changed files with 1,604 additions and 1,536 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2b3d5826836ac59f8446281976762d594e55d97e
3c27178a2f2c74f14d90613028e3929e1f06d830
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if (
}
"use strict";

var ReactVersion = "18.3.0-www-classic-996b7d13";
var ReactVersion = "18.3.0-www-classic-2c91e2cd";

// ATTENTION
// When adding new symbols to this file,
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -615,4 +615,4 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-www-modern-b27aac94";
exports.version = "18.3.0-www-modern-a19b7375";
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-profiling.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-www-modern-b5b87391";
exports.version = "18.3.0-www-modern-4c57bafd";

/* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */
if (
Expand Down
18 changes: 13 additions & 5 deletions compiled/facebook-www/ReactDOM-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -4104,10 +4104,7 @@ function restoreControlledInputState(element, props) {
"ReactDOMInput: Mixing React and non-React radio inputs with the " +
"same `name` is not supported."
);
} // We need update the tracked value on the named cousin since the value
// was changed but the input saw no event or value set

updateValueIfChanged(otherNode); // If this is a controlled radio button group, forcing the input that
} // If this is a controlled radio button group, forcing the input that
// was previously checked to update will cause it to be come re-checked
// as appropriate.

Expand All @@ -4121,6 +4118,17 @@ function restoreControlledInputState(element, props) {
otherProps.type,
otherProps.name
);
} // If any updateInput() call set .checked to true, an input in this group
// (often, `rootNode` itself) may have become unchecked

for (var _i = 0; _i < group.length; _i++) {
var _otherNode = group[_i];

if (_otherNode.form !== rootNode.form) {
continue;
}

updateValueIfChanged(_otherNode);
}
}
} // In Chrome, assigning defaultValue to certain input types triggers input validation.
Expand Down Expand Up @@ -34013,7 +34021,7 @@ function createFiberRoot(
return root;
}

var ReactVersion = "18.3.0-www-classic-bb6c815e";
var ReactVersion = "18.3.0-www-classic-bfae45bc";

function createPortal$1(
children,
Expand Down
18 changes: 13 additions & 5 deletions compiled/facebook-www/ReactDOM-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -3946,10 +3946,7 @@ function restoreControlledInputState(element, props) {
"ReactDOMInput: Mixing React and non-React radio inputs with the " +
"same `name` is not supported."
);
} // We need update the tracked value on the named cousin since the value
// was changed but the input saw no event or value set

updateValueIfChanged(otherNode); // If this is a controlled radio button group, forcing the input that
} // If this is a controlled radio button group, forcing the input that
// was previously checked to update will cause it to be come re-checked
// as appropriate.

Expand All @@ -3963,6 +3960,17 @@ function restoreControlledInputState(element, props) {
otherProps.type,
otherProps.name
);
} // If any updateInput() call set .checked to true, an input in this group
// (often, `rootNode` itself) may have become unchecked

for (var _i = 0; _i < group.length; _i++) {
var _otherNode = group[_i];

if (_otherNode.form !== rootNode.form) {
continue;
}

updateValueIfChanged(_otherNode);
}
}
} // In Chrome, assigning defaultValue to certain input types triggers input validation.
Expand Down Expand Up @@ -33858,7 +33866,7 @@ function createFiberRoot(
return root;
}

var ReactVersion = "18.3.0-www-modern-c7a0d8fc";
var ReactVersion = "18.3.0-www-modern-f84a446a";

function createPortal$1(
children,
Expand Down
Loading

0 comments on commit 0aa4101

Please sign in to comment.