Many form components don't support uncontrolled usage

[manzoorwanijk]

Description

I was trying to play around with @wordpress/components for a plugin settings page in React for a demo for a WordCamp talk and I immediately noticed some problems in the current state of the components needed to build a form.

The package and some of the components mentioned here have been around for years but still do not support the fundamental web standards. Using the components as uncontrolled is nearly impossible. One is forced to make the components controlled which is not very performant for large and complex forms.

Also, someone may want to stick to web fundamentals for building forms for plugin/theme settings pages.

Someone would argue that we:

Avoid the "controlled to uncontrolled" warning by forcing the internal <input /> element to be always in controlled mode

It is not a valid argument because the responsibility of that warning goes to the consumer component and that is how the native form elements work.

Let us start:

Step-by-step reproduction instructions

TextControl

<TextControl
	name="some-name"
	label='Some label'
	defaultValue='Some default value'
/>

Issues:

• TS complains about:

Type '***' is missing the following properties from type '***': onChange, value

• Console error when the input value is changed - Uncaught TypeError: onChange is not a function

TextareaControl

Same as TextControl

__experimentalInputControl

One would argue that we have a better replacement for TextControl in the name of __experimentalInputControl but it again fails:

<InputControl
	name="some-name"
	label='Some label'
	defaultValue='Some default value'
/>

Issues

• Doesn't use the default value
• Console

Warning

Styled(input) contains an input of type undefined with both value and defaultValue props. Input elements must be either controlled or uncontrolled (specify either the value prop, or the defaultValue prop, but not both). Decide between using a controlled or uncontrolled input element and remove one of these props.

ToggleControl

<ToggleControl
	label="Fixed Background"
	defaultChecked={false}
/>

Issues

• TS complains

Property 'onChange' is missing in type '***' but required in type '***'

• Console says: Uncaught TypeError: onChange is not a function
• The toggled state doesn't reflect

CheckboxControl

<CheckboxControl
	name='some-name'
	value='some-value'
	defaultChecked={false}
	label='Some label'
/>

Issues

• The checked state here is messed up after fixing onChange issue.
  
• Other issues are the same as those of ToggleControl

RadioControl

<RadioControl
	name="some-name"
	label={'Something'}
	options={[
		{ value: '1', label: 'One' },
		{ value: '2', label: 'Two', defaultChecked: true },
	]}
/>

Issues

• Console says: Uncaught TypeError: onChange is not a function
• Doesn't support defaultChecked for the options prop or in the component props

Screenshots, screen recording, code snippet

Codesandbox: https://codesandbox.io/p/sandbox/wordpress-components-issues-h5lgy8?file=%2Fsrc%2FApp.tsx

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[bph]

@ciampo @mirka this issue was part of the Extensibility Issues Triage today and it wasn't clear if this was still on your radar. It is a developer experience-related issue.

... removed it from the IGE board as it's not a blocker for extensibility.

[ciampo]

Hey @bph , thank you for the ping!

It is indeed true that many components only work in controlled mode. They were built this way to follow the needs of the editor, where all values of those controls are tied to the editor and block settings.

Ideally, we'd like consumers of the package to be able to use all components in controlled or uncontrolled mode. Unfortunately, it's not always an easy task, since some of the changes that are needed would introduce backward compatibility issues.

Since @mirka and I started to work on the components package, we recognised this gap and tried to improve it where possible — for example, we recently enhanced controlled/uncontrolled support for ToggleGroupControl, and we added controlled mode to TabPanel (by replacing it with a new component, Tabs).

Given all of the above, this doesn't represent a priority at the moment, although we're always open to find ways to improve the situation a step at the time.

[manzoorwanijk]

Even the new components like FormToggle don't work in uncontrolled mode 🤦

[ciampo]

new components like FormToggle

AFAIK FormToggle was added to the package 6 years ago, and prior to that was already part of the Gutenberg repository — so I wouldn't call it "new" necessarily.

As mentioned above, we can't do much about existing components, because it would often require a non-trivial amount of changes (usually breaking) and it would introduce API inconsistencies too.

The good news is that we are aware of this need, and we're making sure that new components (and new versions of existing components) support both controlled and uncontrolled mode.

[manzoorwanijk]

Unfortunately, it's not always an easy task, since some of the changes that are needed would introduce backward compatibility issues.

As mentioned above, we can't do much about existing components, because it would often require a non-trivial amount of changes (usually breaking) and it would introduce API inconsistencies too.

I don't agree with that at all.

1. Why would making an input support uncontrolled mode break backward compatibility?
2. Won't you still be able to make it controlled if you want?

I'm willing to work on those "non-trivial amount of changes" if the team is up for it.

The point is, instead of creating new components for "the" basic use case, why not fix the existing ones?

[mirka]

The breaking changes we're worrying about is around the fact that some components support an implicit controlled/uncontrolled switch through a single value prop (#47473).

If we adopt this pattern on components that don't currently have it, it would introduce behavior changes in controlled usage when the consumer passes an undefined value (to explicity signify an "empty" value, rather than to signify uncontrolled usage).

But I'm starting to feel strongly that not supporting uncontrolled mode is inacceptable, so I think we should consider using the defaultValue pattern if necessary. We'll have to investigate, but it is possible that this can be added onto existing components in a non-breaking and consistent way. Even on components that already have the implicit mode switching on value.

We will first need to audit the current form components to understand the feasibility and impact. I believe we would need to know:

• Does it support uncontrolled?
• Does it have roughly equivalent unit tests for both controlled/uncontrolled? (If not, uncontrolled support may not be "complete")
• How does it support uncontrolled? (useControlledValue, useControlledState, or custom implementation)

Then, we can maybe test out adding a defaultValue prop on a component that already has the mode switching, ideally with already good test coverage, and see if there are any blockers.

@WordPress/gutenberg-components Any thoughts about this plan of action?

[tyxla]

@mirka your plan makes sense to me. I'd love to have a better idea what we're fighting with here, and the audit you proposed should help with that, and answer questions like:

• How many and which components do we need to change
• How difficult and potentially breaking would it be to support uncontrolled versions of each of those components

And I agree with this sentiment:

But I'm starting to feel strongly that not supporting uncontrolled mode is inacceptable

mostly because the @wordpress/components package is turning into a general-purpose admin library to cover all the modern admin design use cases, and we need our components to be more flexible. With that in mind, supporting both controlled and uncontrolled versions sounds like an essential use case.

[ciampo]

Hey @manzoorwanijk, as Lena also explained, some older components were written only to support controlled mode, which means that they don't offer a way to easily differentiate "empty value but controlled" from "uncontrolled value" (see this issue that Lena also linked above).

As an example, you can take a look at the utility function that I wrote to "patch" ToggleGroupControl's behavior in a way that preserved backwards compatibility:

gutenberg/packages/components/src/toggle-group-control/toggle-group-control/utils.ts

Lines 21 to 50 in 1e11103

	export function useComputeControlledOrUncontrolledValue(
	valueProp: ValueProp
	): { value: ValueProp; defaultValue: ValueProp } {
	const isInitialRender = useRef( true );
	const prevValueProp = usePrevious( valueProp );
	const prevIsControlled = useRef( false );
	useEffect( () => {
	if ( isInitialRender.current ) {
	isInitialRender.current = false;
	}
	}, [] );
	// Assume the component is being used in controlled mode on the first re-render
	// that has a different `valueProp` from the previous render.
	const isControlled =
	prevIsControlled.current ||
	( ! isInitialRender.current && prevValueProp !== valueProp );
	useEffect( () => {
	prevIsControlled.current = isControlled;
	}, [ isControlled ] );
	if ( isControlled ) {
	// When in controlled mode, use `''` instead of `undefined`
	return { value: valueProp ?? '', defaultValue: undefined };
	}
	// When in uncontrolled mode, the `value` should be intended as the initial value
	return { value: undefined, defaultValue: valueProp };
	}

---

@tyxla and @mirka , I agree that supporting controlled/uncontrolled mode for a component should be a commodity in 2024, and is quite essential for a component library that wants to become more general purpose.

We should definitely gather some extra info, but my gut feeling tells me that we may need to introduce some breaking changes if we want to support the canonical value / defaultValue / onChange pattern, in particular around how passing an undefined value affects components.

[manzoorwanijk]

Thank you for the explanation. Yes, I think breaking change might be a step in the right direction.

[mirka]

My hope is that it turns out we can do it without breaking changes though. Leave the value behavior as is on all components, and using the new defaultValue prop would be an opt in to standard uncontrolled behavior that ignores value.

@manzoorwanijk If you're willing to do some initial investigation, it would be great if we could have a list of control components with the following information:

• Does it support uncontrolled?
• Does it have roughly equivalent unit tests for both controlled/uncontrolled? (If not, uncontrolled support may not be "complete")
• How does it support uncontrolled? (useControlledValue, useControlledState, or custom implementation)

If you don't have the bandwidth, even just the first bullet point would be helpful!