Skip to content

Block Fields: Remove normalization code and tidy up #74532

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

Merged
talldan merged 12 commits into from
Jan 13, 2026
Merged

Block Fields: Remove normalization code and tidy up #74532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
da36eac
Remove normalization functions
talldan Jan 12, 2026
f61e874
Update link field to remove mapping
talldan Jan 12, 2026
b270364
Update media field
talldan Jan 12, 2026
c392621
Update rich text
talldan Jan 12, 2026
6092de7
Fix allowedTypes
talldan Jan 12, 2026
8f36fdd
Remove defaultValues config and config prop that seems to not be working
talldan Jan 12, 2026
5f14aa3
Improve clarity of createConfiguredControl
talldan Jan 12, 2026
8c5f600
Move event handler to after early return
talldan Jan 12, 2026
6a839db
Remove hideLabelFromVision prop
talldan Jan 12, 2026
5415716
Remove use of mapping in media onChange
talldan Jan 12, 2026
9b442f5
Add getValue/setValue comment
talldan Jan 12, 2026
8dcc153
Fix use of config in MediaThumbnail
talldan Jan 13, 2026
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
256 changes: 49 additions & 207 deletions packages/block-editor/src/hooks/block-fields/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,135 +39,20 @@ const CONTROLS = {
* Creates a configured control component that wraps a custom control
* and passes configuration as props.
*
* @param {Object} config - The control configuration
* @param {string} config.control - The control type (key in CONTROLS map)
* @param {Component} ControlComponent The React component for the control.
* @param {string} type The type of control.
* @param {Object} config The control configuration passed as a prop.
*
* @return {Function} A wrapped control component
*/
function createConfiguredControl( config ) {
const { control, ...controlConfig } = config;
const ControlComponent = CONTROLS[ control ];

function createConfiguredControl( ControlComponent, type, config ) {
if ( ! ControlComponent ) {
throw new Error( `Control type "${ control }" not found` );
throw new Error( `Control type "${ type }" not found` );
}

return function ConfiguredControl( props ) {
return <ControlComponent { ...props } config={ controlConfig } />;
};
}

/**
* Normalize a media value to a canonical structure.
* Only includes properties that are present in the field's mapping (if provided).
*
* @param {Object} value - The mapped value from the block attributes (with canonical keys)
* @param {Object} fieldDef - Optional field definition containing the mapping
* @return {Object} Normalized media value with canonical properties
*/
function normalizeMediaValue( value, fieldDef ) {
const defaults = {
id: null,
url: '',
caption: '',
alt: '',
type: 'image',
poster: '',
featuredImage: false,
link: '',
};

const result = {};

// If there's a mapping, only include properties that are in it
if ( fieldDef?.mapping ) {
Object.keys( fieldDef.mapping ).forEach( ( key ) => {
result[ key ] = value?.[ key ] ?? defaults[ key ] ?? '';
} );
return result;
}

// Without mapping, include all default properties
Object.keys( defaults ).forEach( ( key ) => {
result[ key ] = value?.[ key ] ?? defaults[ key ];
} );
return result;
}

/**
* Denormalize a media value from canonical structure back to mapped keys.
* Only includes properties that are present in the field's mapping.
*
* @param {Object} value - The normalized media value
* @param {Object} fieldDef - The field definition containing the mapping
* @return {Object} Value with only mapped properties
*/
function denormalizeMediaValue( value, fieldDef ) {
if ( ! fieldDef.mapping ) {
return value;
}

const result = {};
Object.entries( fieldDef.mapping ).forEach( ( [ key ] ) => {
if ( key in value ) {
result[ key ] = value[ key ];
}
} );
return result;
}

/**
* Normalize a link value to a canonical structure.
* Only includes properties that are present in the field's mapping (if provided).
*
* @param {Object} value - The mapped value from the block attributes (with canonical keys)
* @param {Object} fieldDef - Optional field definition containing the mapping
* @return {Object} Normalized link value with canonical properties
*/
function normalizeLinkValue( value, fieldDef ) {
const defaults = {
url: '',
rel: '',
linkTarget: '',
destination: '',
return <ControlComponent { ...props } config={ config } />;
};

const result = {};

// If there's a mapping, only include properties that are in it
if ( fieldDef?.mapping ) {
Object.keys( fieldDef.mapping ).forEach( ( key ) => {
result[ key ] = value?.[ key ] ?? defaults[ key ] ?? '';
} );
return result;
}

// Without mapping, include all default properties
Object.keys( defaults ).forEach( ( key ) => {
result[ key ] = value?.[ key ] ?? defaults[ key ];
} );
return result;
}

/**
* Denormalize a link value from canonical structure back to mapped keys.
* Only includes properties that are present in the field's mapping.
*
* @param {Object} value - The normalized link value
* @param {Object} fieldDef - The field definition containing the mapping
* @return {Object} Value with only mapped properties
*/
function denormalizeLinkValue( value, fieldDef ) {
if ( ! fieldDef.mapping ) {
return value;
}

const result = {};
Object.entries( fieldDef.mapping ).forEach( ( [ key ] ) => {
if ( key in value ) {
result[ key ] = value[ key ];
}
} );
return result;
}

/**
Expand Down Expand Up @@ -218,100 +103,63 @@ function BlockFields( {
}

return blockTypeFields.map( ( fieldDef ) => {
const ControlComponent = CONTROLS[ fieldDef.type ];

const defaultValues = {};
if ( fieldDef.mapping && blockType?.attributes ) {
Object.entries( fieldDef.mapping ).forEach(
( [ key, attrKey ] ) => {
defaultValues[ key ] =
blockType.attributes[ attrKey ]?.defaultValue ??
undefined;
}
);
}

const field = {
id: fieldDef.id,
label: fieldDef.label,
type: fieldDef.type, // Use the field's type; DataForm will use built-in or custom Edit
config: { ...fieldDef.args, defaultValues },
hideLabelFromVision: fieldDef.id === 'content',
// getValue and setValue handle the mapping to block attributes
getValue: ( { item } ) => {
if ( fieldDef.mapping ) {
// Extract mapped properties from the block attributes
const mappedValue = {};
Object.entries( fieldDef.mapping ).forEach(
( [ key, attrKey ] ) => {
mappedValue[ key ] = item[ attrKey ];
}
);
};

// Normalize to canonical structure based on field type
if ( fieldDef.type === 'media' ) {
return normalizeMediaValue( mappedValue, fieldDef );
// If the field defines a `mapping`, then custom `getValue` and `setValue`
// implementations are provided.
// These functions map from the inconsistent attribute keys found on blocks
// to consistent keys that the field can use internally (and back again).
// When `mapping` isn't provided, we can use the field API's default
// implementation of these functions.
if ( fieldDef.mapping ) {
Comment on lines +112 to +118
Copy link
Contributor Author

@talldan talldan Jan 12, 2026

Choose a reason for hiding this comment

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

Only add getValue / setValue when needed now, which is when there's a mapping.

Also added a comment to explain this.

field.getValue = ( { item } ) => {
// Extract mapped properties from the block attributes
const mappedValue = {};
Object.entries( fieldDef.mapping ).forEach(
( [ key, attrKey ] ) => {
mappedValue[ key ] = item[ attrKey ];
}
if ( fieldDef.type === 'link' ) {
return normalizeLinkValue( mappedValue, fieldDef );
}

// For other types, return as-is
return mappedValue;
}
// For simple id-based fields, use the id as the attribute key
return item[ fieldDef.id ];
},
setValue: ( { item, value } ) => {
if ( fieldDef.mapping ) {
// Denormalize from canonical structure back to mapped keys
let denormalizedValue = value;
if ( fieldDef.type === 'media' ) {
denormalizedValue = denormalizeMediaValue(
value,
fieldDef
);
} else if ( fieldDef.type === 'link' ) {
denormalizedValue = denormalizeLinkValue(
value,
fieldDef
);
);
return mappedValue;
};
field.setValue = ( { value } ) => {
const attributeUpdates = {};
Object.entries( fieldDef.mapping ).forEach(
( [ key, attrKey ] ) => {
attributeUpdates[ attrKey ] = value[ key ];
}

// Build an object with all mapped attributes
const updates = {};
Object.entries( fieldDef.mapping ).forEach(
( [ key, attrKey ] ) => {
// If key is explicitly in value, use it (even if undefined to allow clearing)
// Otherwise, preserve the old value
if ( key in denormalizedValue ) {
updates[ attrKey ] =
denormalizedValue[ key ];
} else {
updates[ attrKey ] = item[ attrKey ];
}
}
);
return updates;
}
// For simple id-based fields, use the id as the attribute key
return { [ fieldDef.id ]: value };
},
};
);
return attributeUpdates;
};
}

// Only add custom Edit component if one exists for this type
const ControlComponent = CONTROLS[ fieldDef.type ];
if ( ControlComponent ) {
// Use EditConfig pattern: Edit is an object with control type and config props
field.Edit = createConfiguredControl( {
control: fieldDef.type,
clientId,
fieldDef,
} );
field.Edit = createConfiguredControl(
ControlComponent,
fieldDef.type,
{
clientId,
fieldDef,
}
);
}

return field;
} );
}, [ blockTypeFields, blockType?.attributes, clientId ] );
}, [ blockTypeFields, clientId ] );

if ( ! blockTypeFields?.length ) {
// TODO - we might still want to show a placeholder for blocks with no fields.
// for example, a way to select the block.
return null;
}
Comment on lines +158 to +162
Copy link
Contributor Author

@talldan talldan Jan 12, 2026

Choose a reason for hiding this comment

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

Moved this early return a little bit earlier.


const handleToggleField = ( fieldId ) => {
setForm( ( prev ) => {
Expand All @@ -329,12 +177,6 @@ function BlockFields( {
} );
};

if ( ! blockTypeFields?.length ) {
// TODO - we might still want to show a placeholder for blocks with no fields.
// for example, a way to select the block.
return null;
}

return (
<div className="block-editor-block-fields__container">
<div className="block-editor-block-fields__header">
Expand Down
52 changes: 13 additions & 39 deletions packages/block-editor/src/hooks/block-fields/link/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,6 @@ export default function Link( { data, field, onChange, config = {} } ) {
isControl: true,
} );
const { fieldDef } = config;
const updateAttributes = ( newValue ) => {
const mappedChanges = field.setValue( { item: data, value: newValue } );
onChange( mappedChanges );
};

const value = field.getValue( { item: data } );
const url = value?.url;
const rel = value?.rel || '';
Expand Down Expand Up @@ -145,30 +140,12 @@ export default function Link( { data, field, onChange, config = {} } ) {
...newValues,
} );

// Build update object dynamically based on what's in the mapping
const updateValue = { ...value };

if ( fieldDef?.mapping ) {
Object.keys( fieldDef.mapping ).forEach(
( key ) => {
if ( key === 'href' || key === 'url' ) {
updateValue[ key ] =
updatedAttrs.url;
} else if ( key === 'rel' ) {
updateValue[ key ] =
updatedAttrs.rel;
} else if (
key === 'target' ||
key === 'linkTarget'
) {
updateValue[ key ] =
updatedAttrs.linkTarget;
}
}
);
}
Comment on lines -151 to -169
Copy link
Contributor Author

@talldan talldan Jan 12, 2026

Choose a reason for hiding this comment

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

This code isn't required as the setValues function already does this by iterating over mapping.


updateAttributes( updateValue );
onChange(
field.setValue( {
item: data,
Copy link
Member

@ramonjd ramonjd Jan 13, 2026

Choose a reason for hiding this comment

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

Looking at field.setValue = ( { value } ) => { in packages/block-editor/src/hooks/block-fields/index.js above... does item need to passed?

It's done in several places, so I assume so and (?) but just checking since the implementation appears different to getValue.

Copy link
Contributor Author

@talldan talldan Jan 13, 2026

Choose a reason for hiding this comment

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

I kept it as the type requires it:

gutenberg/packages/dataviews/src/types/field-api.ts

Line 271 in cf3eaf2

setValue?: ( args: { item: Item; value: any } ) => DeepPartial< Item >;

Though it's not TS code, it still feels like good practice.

value: updatedAttrs,
} )
);
} }
onRemove={ () => {
// Remove all link-related properties based on what's in the mapping
Expand All @@ -177,20 +154,17 @@ export default function Link( { data, field, onChange, config = {} } ) {
if ( fieldDef?.mapping ) {
Object.keys( fieldDef.mapping ).forEach(
( key ) => {
if (
key === 'href' ||
key === 'url' ||
key === 'rel' ||
key === 'target' ||
key === 'linkTarget'
) {
removeValue[ key ] = undefined;
}
Comment on lines -180 to -188
Copy link
Contributor Author

@talldan talldan Jan 12, 2026

Choose a reason for hiding this comment

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

The if statement isn't needed as mapping should already have the correct key names.

removeValue[ key ] = undefined;
}
);
}

updateAttributes( removeValue );
onChange(
field.setValue( {
item: data,
value: removeValue,
} )
);
} }
/>
</Popover>
Expand Down
Loading
Loading