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

Lodash: Refactor away from _.set() in global styles #52279

Merged
merged 1 commit into from
Jul 10, 2023
Merged
Changes from all 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
32 changes: 13 additions & 19 deletions packages/block-editor/src/components/global-styles/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import fastDeepEqual from 'fast-deep-equal/es6';
import { get, set } from 'lodash';
import { get } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -16,6 +16,7 @@ import { _x } from '@wordpress/i18n';
* Internal dependencies
*/
import { getValueFromVariable, getPresetVariableFromValue } from './utils';
import { setImmutably } from '../../utils/object';
import { GlobalStylesContext } from './context';
import { unlock } from '../../lock-unlock';

Expand Down Expand Up @@ -108,15 +109,15 @@ export function useGlobalSetting( propertyPath, blockName, source = 'all' ) {
);
}

const result = {};
let result = {};
VALID_SETTINGS.forEach( ( setting ) => {
const value =
get(
configToUse,
`settings${ appendedBlockPath }.${ setting }`
) ?? get( configToUse, `settings.${ setting }` );
if ( value ) {
set( result, setting, value );
result = setImmutably( result, setting.split( '.' ), value );
Copy link
Member Author

Choose a reason for hiding this comment

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

Pretty straightforward to trace - result is already declared to be an object, and setting is already a string, which is handled just fine by setImmutably once we split it to an array path.

}
} );
return result;
Expand All @@ -130,13 +131,9 @@ export function useGlobalSetting( propertyPath, blockName, source = 'all' ) {
] );

const setSetting = ( newValue ) => {
setUserConfig( ( currentConfig ) => {
// Deep clone `currentConfig` to avoid mutating it later.
const newUserConfig = JSON.parse( JSON.stringify( currentConfig ) );
set( newUserConfig, contextualPath, newValue );

return newUserConfig;
} );
setUserConfig( ( currentConfig ) =>
Copy link
Member Author

Choose a reason for hiding this comment

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

Simplifying because we don't need to deep clone - setImmutably will already do that.

setImmutably( currentConfig, contextualPath.split( '.' ), newValue )
Copy link
Member Author

Choose a reason for hiding this comment

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

We just have to split contextualPath to an array path to have it properly handled by setImmutably.

);
};

return [ settingValue, setSetting ];
Expand All @@ -160,12 +157,10 @@ export function useGlobalStyle(
: `styles.blocks.${ blockName }${ appendedPath }`;

const setStyle = ( newValue ) => {
setUserConfig( ( currentConfig ) => {
// Deep clone `currentConfig` to avoid mutating it later.
const newUserConfig = JSON.parse( JSON.stringify( currentConfig ) );
set(
newUserConfig,
finalPath,
setUserConfig( ( currentConfig ) =>
Copy link
Member Author

Choose a reason for hiding this comment

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

Simplifying because we don't need to deep clone - setImmutably will already do that.

setImmutably(
currentConfig,
finalPath.split( '.' ),
Copy link
Member Author

Choose a reason for hiding this comment

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

We just have to split finalPath to an array path to have it properly handled by setImmutably.

shouldDecodeEncode
? getPresetVariableFromValue(
mergedConfig.settings,
Expand All @@ -174,9 +169,8 @@ export function useGlobalStyle(
newValue
)
: newValue
);
return newUserConfig;
} );
)
);
};

let rawResult, result;
Expand Down