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.

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

Indicate nested paths on __experimentalSaveSpecifiedEntityEdits #54161

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
11 changes: 5 additions & 6 deletions packages/core-data/src/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import deprecated from '@wordpress/deprecated';
/**
* Internal dependencies
*/
import { getNestedValue, setNestedValue } from './utils';
import { receiveItems, removeItems, receiveQueriedItems } from './queried-data';
import { getOrLoadEntitiesConfig, DEFAULT_ENTITY_KEY } from './entities';
import { createBatch } from './batch';
Expand Down Expand Up @@ -779,7 +780,7 @@ export const saveEditedEntityRecord =
* @param {string} kind Kind of the entity.
* @param {string} name Name of the entity.
* @param {Object} recordId ID of the record.
* @param {Array} itemsToSave List of entity properties to save.
* @param {Array} itemsToSave List of entity properties or property paths to save.
* @param {Object} options Saving options.
*/
export const __experimentalSaveSpecifiedEntityEdits =
Expand All @@ -794,10 +795,9 @@ export const __experimentalSaveSpecifiedEntityEdits =
recordId
);
const editsToSave = {};
for ( const edit in edits ) {
if ( itemsToSave.some( ( item ) => item === edit ) ) {
editsToSave[ edit ] = edits[ edit ];
}

for ( const item of itemsToSave ) {
setNestedValue( editsToSave, item, getNestedValue( edits, item ) );
}

const configs = await dispatch( getOrLoadEntitiesConfig( kind ) );
Expand All @@ -814,7 +814,6 @@ export const __experimentalSaveSpecifiedEntityEdits =
if ( recordId ) {
editsToSave[ entityIdKey ] = recordId;
}

return await dispatch.saveEntityRecord(
kind,
name,
Expand Down
27 changes: 27 additions & 0 deletions packages/core-data/src/utils/get-nested-value.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* Helper util to return a value from a certain path of the object.
* Path is specified as either:
* - a string of properties, separated by dots, for example: "x.y".
* - an array of properties, for example `[ 'x', 'y' ]`.
* You can also specify a default value in case the result is nullish.
*
* @param {Object} object Input object.
* @param {string|Array} path Path to the object property.
* @param {*} defaultValue Default value if the value at the specified path is nullish.
* @return {*} Value of the object property at the specified path.
*/
export default function getNestedValue( object, path, defaultValue ) {
Copy link
Member

Choose a reason for hiding this comment

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

What a bummer that we're introducing yet another version of that utility function. While migrating away from Lodash we had to introduce (almost duplicate) a few of those in various packages, because this was the very last function to remove and we were willing to make some temporary compromises. But ideally, we should unify them all and avoid to introduce new ones if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tyxla I agree. I think we can merge this and follow up with the creation of the package after.Right now we need this util to launch the Font Library. This change is in use here: #53884

if (
! object ||
typeof object !== 'object' ||
( typeof path !== 'string' && ! Array.isArray( path ) )
) {
return object;
}
const normalizedPath = Array.isArray( path ) ? path : path.split( '.' );
matiasbenedetto marked this conversation as resolved.
Show resolved Hide resolved
let value = object;
normalizedPath.forEach( ( fieldName ) => {
value = value?.[ fieldName ];
} );
return value !== undefined ? value : defaultValue;
matiasbenedetto marked this conversation as resolved.
Show resolved Hide resolved
}
1 change: 1 addition & 0 deletions packages/core-data/src/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ export { default as replaceAction } from './replace-action';
export { default as withWeakMapCache } from './with-weak-map-cache';
export { default as isRawAttribute } from './is-raw-attribute';
export { default as setNestedValue } from './set-nested-value';
export { default as getNestedValue } from './get-nested-value';
18 changes: 12 additions & 6 deletions packages/core-data/src/utils/set-nested-value.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
* Arrays are created for missing index properties while objects are created
* for all other missing properties.
*
* Path is specified as either:
* - a string of properties, separated by dots, for example: "x.y".
* - an array of properties, for example `[ 'x', 'y' ]`.
*
* This function intentionally mutates the input object.
*
* Inspired by _.set().
Expand All @@ -12,24 +16,26 @@
*
* @todo Needs to be deduplicated with its copy in `@wordpress/edit-site`.
*
* @param {Object} object Object to modify
* @param {Array} path Path of the property to set.
* @param {*} value Value to set.
* @param {Object} object Object to modify
* @param {Array|string} path Path of the property to set.
* @param {*} value Value to set.
*/
export default function setNestedValue( object, path, value ) {
if ( ! object || typeof object !== 'object' ) {
return object;
}

path.reduce( ( acc, key, idx ) => {
const normalizedPath = Array.isArray( path ) ? path : path.split( '.' );
Copy link
Member

Choose a reason for hiding this comment

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

Note that when introducing setNestedValue() we intentionally kept it array-only. I'd prefer that because it helps keep a simpler and more predictable utility function; while if we support multiple formats and types, it more or less emulates the complex Lodash behavior, which is something we want to avoid.

Also, keep in mind I'm not saying that we shouldn't use the dot notation (x.y.z). I'm just saying that the normalization part (string to array) can be done outside of the utility function, at the place where we're utilizing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could we work on this as a follow-up when we make this a new package?


normalizedPath.reduce( ( acc, key, idx ) => {
if ( acc[ key ] === undefined ) {
if ( Number.isInteger( path[ idx + 1 ] ) ) {
if ( Number.isInteger( normalizedPath[ idx + 1 ] ) ) {
acc[ key ] = [];
} else {
acc[ key ] = {};
}
}
if ( idx === path.length - 1 ) {
if ( idx === normalizedPath.length - 1 ) {
acc[ key ] = value;
}
return acc[ key ];
Expand Down
61 changes: 61 additions & 0 deletions packages/core-data/src/utils/test/get-nested-value.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* Internal dependencies
*/
import getNestedValue from '../get-nested-value';

describe( 'getNestedValue', () => {
it( 'should return the same object unmodified if path is an empty array', () => {
const input = { x: 'y' };
const result = getNestedValue( input, [] );
expect( result ).toEqual( input );
} );

it( 'should the nested value', () => {
matiasbenedetto marked this conversation as resolved.
Show resolved Hide resolved
const input = { x: { y: { z: 123 } } };
const result = getNestedValue( input, [ 'x', 'y', 'z' ] );

expect( result ).toEqual( 123 );
} );

it( 'should return the nested value if the path is a string', () => {
const input = { x: { y: { z: 123 } } };
const result = getNestedValue( input, 'x.y.z' );

expect( result ).toEqual( 123 );
} );

it( 'should return the shallow value', () => {
const input = { x: { y: { z: 123 } } };
const result = getNestedValue( input, 'x' );

expect( result ).toEqual( { y: { z: 123 } } );
} );

it( 'should return the default value if the nested value is undefined', () => {
matiasbenedetto marked this conversation as resolved.
Show resolved Hide resolved
const input = { x: { y: { z: undefined } } };
const result = getNestedValue( input, [ 'x', 'y', 'z' ], 456 );

expect( result ).toEqual( 456 );
} );

it( 'should return the nested value if it is different to undefined', () => {
const input = { x: { y: { z: null } } };
matiasbenedetto marked this conversation as resolved.
Show resolved Hide resolved
const result = getNestedValue( input, 'x.y.z', 456 );

expect( result ).toBeNull();
} );

it( 'should return the default value if the nested value does not exist', () => {
const input = { x: { y: { z: 123 } } };
const result = getNestedValue( input, [ 'x', 'y', 'z1' ], 456 );

expect( result ).toEqual( 456 );
} );

it( 'should return undefined if the nested value does not exist', () => {
const input = { x: { y: { z: 123 } } };
const result = getNestedValue( input, [ 'x', 'y', 'z1' ] );

expect( result ).toBeUndefined();
} );
} );
7 changes: 7 additions & 0 deletions packages/core-data/src/utils/test/set-nested-value.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ describe( 'setNestedValue', () => {
expect( result ).toEqual( { x: { y: { z: 456 } } } );
} );

it( 'should set values at deep level having a string as path', () => {
const input = { x: { y: { z: 123 } } };
const result = setNestedValue( input, 'x.y.z', 456 );

expect( result ).toEqual( { x: { y: { z: 456 } } } );
} );

it( 'should create nested objects if necessary', () => {
const result = setNestedValue( {}, [ 'x', 'y', 'z' ], 123 );

Expand Down