Skip to content

Commit

Permalink
Add cx as a dependency of useMemo across @wordpress/components
Browse files Browse the repository at this point in the history
…package (#38541)

* Add `useCx` storybook example

* Add `cx` as a `useMemo` dependency across the whole `@wordpress/components` package

* Update storybook example

* CHANGELOG

* Add preemptively the eslint disable comment

* Rewrite example to avoid using the `args` array as prop
  • Loading branch information
ciampo authored Feb 7, 2022
1 parent c86a198 commit 6eff9a5
Show file tree
Hide file tree
Showing 25 changed files with 83 additions and 26 deletions.
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
- Fixed typing errors for `ColorPicker` ([#38430](https://github.com/WordPress/gutenberg/pull/38430)).
- Updated destructuring of `Dropdown` props to be TypeScript friendly ([#38431](https://github.com/WordPress/gutenberg/pull/38431)).
- Added `ts-nocheck` to `ColorIndicator` so it can be used in typed components ([#38433](https://github.com/WordPress/gutenberg/pull/38433)).
- Added `cx` as a dependency of `useMemo` across the whole package, in order to recalculate the classnames correctly when a component is rendered across more than one `StyleProvider` ([#38541](https://github.com/WordPress/gutenberg/pull/38541)).

### Enhancements

Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/base-field/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export function useBaseField( props ) {
isInline && styles.inline,
className
),
[ className, controlGroupStyles, hasError, isInline, isSubtle ]
[ className, controlGroupStyles, cx, hasError, isInline, isSubtle ]
);

return {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/card/card-body/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function useCardBody( props ) {
'components-card__body',
className
),
[ className, isShady, size ]
[ className, cx, isShady, size ]
);

return {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/card/card-divider/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export function useCardDivider( props ) {
'components-card__divider',
className
),
[ className ]
[ className, cx ]
);

return {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/card/card-footer/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function useCardFooter( props ) {
'components-card__footer',
className
),
[ className, isBorderless, isShady, size ]
[ className, cx, isBorderless, isShady, size ]
);

return {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/card/card-header/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function useCardHeader( props ) {
'components-card__header',
className
),
[ className, isBorderless, isShady, size ]
[ className, cx, isBorderless, isShady, size ]
);

return {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/card/card-media/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export function useCardMedia( props ) {
'components-card__media',
className
),
[ className ]
[ className, cx ]
);

return {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/card/card/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function Card( props, forwardedRef ) {

const elevationClassName = useMemo(
() => cx( css( { borderRadius: elevationBorderRadius } ) ),
[ elevationBorderRadius ]
[ cx, elevationBorderRadius ]
);

const contextProviderValue = useMemo( () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/card/card/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export function useCard( props ) {
isRounded && styles.rounded,
className
);
}, [ className, isBorderless, isRounded ] );
}, [ className, cx, isBorderless, isRounded ] );

const surfaceProps = useSurface( { ...otherProps, className: classes } );

Expand Down
1 change: 1 addition & 0 deletions packages/components/src/elevation/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ export function useElevation( props ) {
active,
borderRadius,
className,
cx,
focus,
hover,
isInteractive,
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/flex/flex/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ export function useFlex( props ) {
}, [
align,
className,
cx,
direction,
expanded,
gap,
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/grid/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export default function useGrid( props ) {
alignment,
className,
columnGap,
cx,
gap,
gridTemplateColumns,
gridTemplateRows,
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/item-group/item/hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export function useItem( props: WordPressComponentProps< ItemProps, 'div' > ) {
spacedAround && styles.spacedAround,
className
),
[ as, className, size, spacedAround ]
[ as, className, cx, size, spacedAround ]
);

const wrapperClassName = cx( styles.itemWrapper );
Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/item-group/stories/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ const ItemWithChevron = ( {

const itemClassName = useMemo(
() => cx( ! alwaysVisible && appearingChevron, className ),
[ alwaysVisible, className ]
[ alwaysVisible, className, cx ]
);

const chevronIconClassName = useMemo(
Expand All @@ -155,7 +155,7 @@ const ItemWithChevron = ( {
fill: currentColor;
transform: ${ isRtlLayout ? 'scaleX( -100% )' : 'none' };
` ),
[ isRtlLayout ]
[ cx, isRtlLayout ]
);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function NavigatorProvider(
const classes = useMemo(
// Prevents horizontal overflow while animating screen transitions
() => cx( css( { overflowX: 'hidden' } ), className ),
[ className ]
[ className, cx ]
);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ function NavigatorScreen( props: Props, forwardedRef: Ref< any > ) {
} ),
className
),
[ className ]
[ className, cx ]
);

// This flag is used to only apply the focus on mount when the actual path changes.
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/scrollable/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export function useScrollable( props ) {
scrollDirection === 'auto' && styles.scrollAuto,
className
),
[ className, scrollDirection, smoothScroll ]
[ className, cx, scrollDirection, smoothScroll ]
);

return { ...otherProps, className: classes };
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/surface/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export function useSurface( props ) {
borderRight,
borderTop,
className,
cx,
variant,
] );

Expand Down
1 change: 1 addition & 0 deletions packages/components/src/text/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ export default function useText( props ) {
align,
className,
color,
cx,
display,
isBlock,
isCaption,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ function ToggleGroupControl(
'medium',
className
),
[ className, isBlock ]
[ className, cx, isBlock ]
);
return (
<BaseControl help={ help }>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ export function useToolsPanelHeader(
const cx = useCx();
const classes = useMemo( () => {
return cx( styles.ToolsPanelHeader, className );
}, [ className ] );
}, [ className, cx ] );

const dropdownMenuClassName = useMemo( () => {
return cx( styles.DropdownMenu );
}, [] );
}, [ cx ] );

const headingClassName = useMemo( () => {
return cx( styles.ToolsPanelHeading );
}, [] );
}, [ cx ] );

const {
menuItems,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ export function useToolsPanelItem(
isShown,
shouldRenderPlaceholder,
className,
cx,
firstDisplayedItem,
lastDisplayedItem,
__experimentalFirstVisibleItemClass,
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/tools-panel/tools-panel/hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ export function useToolsPanel(
}, [
areAllOptionalControlsHidden,
className,
cx,
hasInnerWrapper,
menuItems,
] );
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/truncate/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export default function useTruncate( props ) {
shouldTruncate && !! numberOfLines && sx.numberOfLines,
className
);
}, [ className, numberOfLines, shouldTruncate ] );
}, [ className, cx, numberOfLines, shouldTruncate ] );

return { ...otherProps, className: classes, children: truncatedContent };
}
63 changes: 56 additions & 7 deletions packages/components/src/utils/hooks/stories/use-cx.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { css } from '@emotion/react';
* WordPress dependencies
*/
import { __unstableIframe as Iframe } from '@wordpress/block-editor';
import { useMemo } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -22,9 +23,29 @@ export default {
title: 'Components (Experimental)/useCx',
};

const Example = ( { args, children } ) => {
const Example = ( { serializedStyles, children } ) => {
const cx = useCx();
const classes = cx( ...args );
const classes = cx( serializedStyles );
return <span className={ classes }>{ children }</span>;
};

const ExampleWithUseMemoWrong = ( { serializedStyles, children } ) => {
const cx = useCx();
// Wrong: using 'useMemo' without adding 'cx' to the dependency list.
// eslint-disable-next-line react-hooks/exhaustive-deps
const classes = useMemo( () => cx( serializedStyles ), [
serializedStyles,
] );
return <span className={ classes }>{ children }</span>;
};

const ExampleWithUseMemoRight = ( { serializedStyles, children } ) => {
const cx = useCx();
// Right: using 'useMemo' with 'cx' listed as a dependency.
const classes = useMemo( () => cx( serializedStyles ), [
cx,
serializedStyles,
] );
return <span className={ classes }>{ children }</span>;
};

Expand All @@ -46,17 +67,17 @@ export const _slotFill = () => {
<StyleProvider document={ document }>
<Iframe>
<Iframe>
<Example args={ [ redText ] }>
<Example serializedStyles={ redText }>
This text is inside an iframe and should be red
</Example>
<Fill name="test-slot">
<Example args={ [ blueText ] }>
<Example serializedStyles={ blueText }>
This text is also inside the iframe, but is
relocated by a slot/fill and should be blue
</Example>
</Fill>
<Fill name="outside-frame">
<Example args={ [ greenText ] }>
<Example serializedStyles={ greenText }>
This text is also inside the iframe, but is
relocated by a slot/fill and should be green
</Example>
Expand All @@ -83,7 +104,7 @@ export const _slotFillSimple = () => {
<SlotFillProvider>
<Iframe>
<Fill name="test-slot">
<Example args={ [ redText ] }>
<Example serializedStyles={ redText }>
This text should be red
</Example>
</Fill>
Expand All @@ -93,13 +114,41 @@ export const _slotFillSimple = () => {
);
};

export const _useMemoBadPractices = () => {
const redText = css`
color: red;
`;
const blueText = css`
color: blue;
`;

return (
<>
<Example serializedStyles={ redText }>
This text should be red
</Example>
<ExampleWithUseMemoRight serializedStyles={ blueText }>
This text should be blue
</ExampleWithUseMemoRight>
<Iframe>
<Example serializedStyles={ redText }>
This text should be red
</Example>
<ExampleWithUseMemoWrong serializedStyles={ blueText }>
This text should be blue but it&apos;s not!
</ExampleWithUseMemoWrong>
</Iframe>
</>
);
};

export const _default = () => {
const redText = css`
color: red;
`;
return (
<Iframe>
<Example args={ [ redText ] }>
<Example serializedStyles={ redText }>
This text is inside an iframe and is red!
</Example>
</Iframe>
Expand Down

0 comments on commit 6eff9a5

Please sign in to comment.