Skip to content

Commit

Permalink
Lodash: Refactor context system provider away from _.merge() (#48036)
Browse files Browse the repository at this point in the history
  • Loading branch information
tyxla committed Feb 15, 2023
1 parent 182eaa1 commit fb92f08
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 4 deletions.
7 changes: 7 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions packages/components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,14 @@
"classnames": "^2.3.1",
"colord": "^2.7.0",
"date-fns": "^2.28.0",
"deepmerge": "^4.3.0",
"dom-scroll-into-view": "^1.2.1",
"downshift": "^6.0.15",
"fast-deep-equal": "^3.1.3",
"framer-motion": "^7.6.1",
"gradient-parser": "^0.1.5",
"highlight-words-core": "^1.2.2",
"is-plain-object": "^5.0.0",
"lodash": "^4.17.21",
"memize": "^1.1.0",
"path-to-regexp": "^6.2.1",
Expand Down
11 changes: 7 additions & 4 deletions packages/components/src/ui/context/context-system-provider.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/**
* External dependencies
*/
import deepmerge from 'deepmerge';
import fastDeepEqual from 'fast-deep-equal/es6';
import { merge } from 'lodash';
import { isPlainObject } from 'is-plain-object';

/**
* WordPress dependencies
Expand Down Expand Up @@ -54,18 +55,20 @@ function useContextSystemBridge( { value } ) {
// `parentContext` will always be memoized (i.e., the result of this hook itself)
// or the default value from when the `ComponentsContext` was originally
// initialized (which will never change, it's a static variable)
// so this memoization will prevent `merge` and `JSON.parse/stringify` from rerunning unless
// so this memoization will prevent `deepmerge()` from rerunning unless
// the references to `value` change OR the `parentContext` has an actual material change
// (because again, it's guaranteed to be memoized or a static reference to the empty object
// so we know that the only changes for `parentContext` are material ones... i.e., why we
// don't have to warn in the `useUpdateEffect` hook above for `parentContext` and we only
// need to bother with the `value`). The `useUpdateEffect` above will ensure that we are
// correctly warning when the `value` isn't being properly memoized. All of that to say
// that this should be super safe to assume that `useMemo` will only run on actual
// changes to the two dependencies, therefore saving us calls to `merge` and `JSON.parse/stringify`!
// changes to the two dependencies, therefore saving us calls to `deepmerge()`!
const config = useMemo( () => {
// Deep clone `parentContext` to avoid mutating it later.
return merge( JSON.parse( JSON.stringify( parentContext ) ), value );
return deepmerge( parentContext ?? {}, value ?? {}, {
isMergeableObject: isPlainObject,
} );
}, [ parentContext, value ] );

return config;
Expand Down

1 comment on commit fb92f08

@github-actions
Copy link

@github-actions github-actions bot commented on fb92f08 Feb 15, 2023

Choose a reason for hiding this comment

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

Flaky tests detected in fb92f08.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4185571061
📝 Reported issues:

Please sign in to comment.