Field API: move validation to the field type

[oandregal]

Part of #73548

What?

This PR refactors validation so that logic lives in the field type and not in the useFormValidity hook.

Why?

By holding the validation logic in the field type, the useFormValidity hook is field type agnostic. This means we can support registering custom field types. See #73548

How?

• The field types have a getIsValid function, as ( field: Field< Item > ) => NormalizedRules< Item >.
• There's a new NormalizedRules type with all the rules. Each rule (required, min, max, etc.) can be:
  • false: if the field does not support this rule
  • an object containing the constraint defined by the field author (e.g., min: 5, required: true) and the validate function (which previously lived in useFormValidate)

Testing Instructions

• Start the local storybook (npm run storybook:dev) and go to "DataViews > DataForm > Validation".
• By using the controls in the right to enable/disable all types of validation, verify it works as before.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: oandregal <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: jorgefilipecosta <[email protected]>
Co-authored-by: ntsekouras <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[oandregal]

This is something we missed doing in #73465

[youknowriad]

This function is a bit weird, how does one combine multiple validation rules. Also should this be called getFieldValidity or getCombinedValidity.

It's also possible that I don't really understand what this function does.

[oandregal]

This function just returns the message to be displayed in the control. It's a single message. It could be refactored to return early if it found a message as well.

[youknowriad]

What happens if the validation fails on multiple constraints, shouldn't we combine the message? Should we clarify in the name that it returns a message

[oandregal]

There's only one message per field, we return early when some constraint failed. This early return prevents triggering multiple network requests (custom validation) when they are not used.

[oandregal]

This isValidCustom function is the same as isValid.custom before. It's just unwrapped. The same for every other field type.

[oandregal]

All this logic now lives in specific is-valid-* rules in the field types.

[youknowriad]

isValid not a great name. I also think that "defining the supported constraints" like the field type does, and defining the constraints for a given field shouldn't be the same type. I'm going to do a separate comment that clarifies all what I have in mind as it's now a bit scattered in multiple comments.

[oandregal]

I think it's time to revisit some Field API prop names, specially now that we want to stabilize parts of it. It's grown organically, it was driven by dozens of people, and it's showing a bit.

[youknowriad]

Now that I understand this a bit better, I'm ready to do an alternative proposal which I think would bring clarity IMO.

Right now this PR doesn't separate between how a field define its constraints, and how a field type define which constraints are supported. It tries to be smart by making the fieldType take a field object and return the used constraints by merging what the field defines as constraint with some generic function that a field type can used to "override/enhance" what the field defines. This works but IMO, is not really clear and makes our architecture a bit cumbersome.

Here's my alternative proposal:

I'm going to take how a list of constraints is defined today

max:
		| false
		| {
				constraint: any;
				validate: (
					item: Item,
					field: NormalizedField< Item >
				) => boolean;
		  };

The ambiguity today, is that this is both how a field configure the constraint but also how it may define how the constraint works (the validation function). Which means in theory, a field can override how "required" works by providing a "validate" function for required. I'm going to propose that we separate these two things. A field only defines the configuration for the constraints:

type ValidationConstraintConfig = any;
type ValidationConstraintConfigs = Record<  string, ValidationConstraintConfig >;
interface Field {
   isValid: ValidationConstraintConfigs;
}

A field type on the other side, doesn't define the "config" of a constraint, it defines the constraint itself, a validation function, a function that takes the item, the configuration of the constraint (potentially the field for extra information) and returns the validation result.

type ValidationConstraint = ( item, field: Field, constraintConfig: ValidationConstraintConfig ) => ValidationResult
}

interface FieldType {
   constraints: ValidationConstraint[]
}

now, come required, minLenght.... These all of the same type ValidationConstraint

const required = ( item, field, constraintConfig ) => {
  if ( !! constraintConfig && field.getValue( item ) === undefined ) {
     return 'field is required'; // Potentially these messages can be extracted out of the validation function into a "messages" property per validation constraint
  }
  return true;
}

// ....

What about "custom". It's the same, it's just another validation constraint

const custom =  ( item, field, constraintConfig ) => {
  return constraintConfig( item, field );
}

I hope this clarifies a bit how I see this evolving and scaling.

[youknowriad]

Ok thinking more, I changed my mind on whether the ValidationConstraintConfigs should use Record< string, any >. I'm ok hard-coding the default constraints that we support to allow us to type their configurations better for instance:

interface ValidationConstraintConfigs {
   required?: boolean;
   min?: number;
   max?:number;
   minLength ?: number;
   maxLength ?: number;
   regex?: string;

   custom?: (field: Field, item: Item): ValidationResult
}

[oandregal]

Which means in theory, a field can override how "required" works by providing a "validate" function for required.

No, it can't. But I think it's nice that it'd be easy to do if we wanted.

Right now this PR doesn't separate between how a field define its constraints, and how a field type define which constraints are supported.

The fields define the constraints:

export type Rules< Item > = {
	required?: boolean;
	elements?: boolean;
	pattern?: string;
	minLength?: number;
	maxLength?: number;
	min?: number;
	max?: number;
	custom?:
		| ( ( item: Item, field: NormalizedField< Item > ) => null | string )
		| ( (
				item: Item,
				field: NormalizedField< Item >
		  ) => Promise< null | string > );
};

And the field types define the constraints and how they are validated:

type NormalizedRule< Item > = {
	constraint: any;
	validate: ( item: Item, field: NormalizedField< Item > ) => boolean;
};
export type NormalizedRules< Item > = {
	required?: NormalizedRule< Item >;
	elements?: NormalizedRule< Item >;
	pattern?: NormalizedRule< Item >;
	minLength?: NormalizedRule< Item >;
	maxLength?: NormalizedRule< Item >;
	min?: NormalizedRule< Item >;
	max?: NormalizedRule< Item >;
	custom?:
		| ( ( item: Item, field: NormalizedField< Item > ) => null | string )
		| ( (
				item: Item,
				field: NormalizedField< Item >
		  ) => Promise< null | string > );
};

So there's already a separation for this. Though, it can be made clearer than it is right now. It relates to this conversation #73642 (comment) I won't be able to push more changes today, but this is the next thing I'll address.

[oandregal]

Sorry for the massive ping, folks 🙏 I had an issue with merge/rebase.

[oandregal]

While testing this, I found an issue with pattern validation for text and url field types — it works fine for any other field type that supports it. Other constraints also work as before.

[oandregal]

This is ready now.