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.

Sign up for GitHub

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

Refactor some controlled component stuff #26573

Merged
merged 14 commits into from
Apr 9, 2023
Merged

Refactor some controlled component stuff #26573

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
087f5e9
Rename controlled component methods
sebmarkbage Apr 7, 2023
f57beb1
Use nullish checks instead of hasOwnProperty which is generally our s…
sebmarkbage Apr 7, 2023
0e0af20
Move init and postInit together to show that this doesn't matter
sebmarkbage Apr 7, 2023
0c9c688
Move checkControlledValueProps to ReactDOMComponent
sebmarkbage Apr 7, 2023
e6f26c3
Fix tests
sebmarkbage Apr 8, 2023
a0a47e5
setDefaultValue is always guarded by a null check
sebmarkbage Apr 9, 2023
1ad8392
Since we just initialized the initial state, we can infer it from the…
sebmarkbage Apr 9, 2023
17abdbe
Move controlled warning to be done by last/next props
sebmarkbage Apr 9, 2023
b3e97c5
Delete wrapperState
sebmarkbage Apr 9, 2023
63b40e5
Textarea only initializes the wrapper state to read it at the same time
sebmarkbage Apr 9, 2023
18a5a9f
Remove wrapperState from select
sebmarkbage Apr 9, 2023
2019ddc
Delete value attribute when defaultValue sets back to null/missing
sebmarkbage Apr 9, 2023
b263fa3
Reset textarea to empty after defaultValue is removed or null
sebmarkbage Apr 9, 2023
91f7014
Rename initX to validateXProps and postInitX to initX
sebmarkbage Apr 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions packages/react-dom-bindings/src/client/ReactDOMInput.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,14 @@ export function postInitInput(
return;
}

const initialValue = toString(node._wrapperState.initialValue);
const defaultValue =
props.defaultValue != null
? toString(getToStringValue(props.defaultValue))
: '';
const initialValue =
props.value != null
? toString(getToStringValue(props.value))
: defaultValue;

// Do not assign value if it is already set. This prevents user text input
// from being lost during SSR hydration.
Expand Down Expand Up @@ -280,9 +287,8 @@ export function postInitInput(
if (disableInputAttributeSyncing) {
// When not syncing the value attribute, assign the value attribute
// directly from the defaultValue React property (when present)
const defaultValue = getToStringValue(props.defaultValue);
if (defaultValue != null) {
node.defaultValue = toString(defaultValue);
if (props.defaultValue != null) {
node.defaultValue = defaultValue;
}
} else {
// Otherwise, the value attribute is synchronized to the property,
Expand All @@ -302,13 +308,20 @@ export function postInitInput(
node.name = '';
}

const defaultChecked =
props.checked != null ? props.checked : props.defaultChecked;
Comment on lines +247 to +248
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems a little confusing that your defaultValue and defaultChecked are not parallel (the latter here incorporates props.checked) but I guess it's fine

const initialChecked =
typeof defaultChecked !== 'function' &&
typeof defaultChecked !== 'symbol' &&
!!defaultChecked;

// The checked property never gets assigned. It must be manually set.
// We don't want to do this when hydrating so that existing user input isn't
// modified
// TODO: I'm pretty sure this is a bug because initialValueTracking won't be
// correct for the hydration case then.
if (!isHydrating) {
node.checked = !!node._wrapperState.initialChecked;
node.checked = !!initialChecked;
}

if (disableInputAttributeSyncing) {
Expand All @@ -327,7 +340,7 @@ export function postInitInput(
// 2. The defaultChecked React property when present
// 3. Otherwise, false
node.defaultChecked = !node.defaultChecked;
node.defaultChecked = !!node._wrapperState.initialChecked;
node.defaultChecked = !!initialChecked;
}

if (name !== '') {
Expand Down