From 511658ce800bff1b478cafc2900f772e5780d732 Mon Sep 17 00:00:00 2001 From: Melloware Date: Sun, 18 Feb 2024 10:08:12 -0500 Subject: [PATCH] Fix #5983: MergeProps refactor and optimize (#5996) --- components/lib/carousel/Carousel.js | 6 +- .../lib/colorpicker/ColorPickerPanel.js | 3 +- components/lib/componentbase/ComponentBase.js | 8 +- components/lib/datatable/BodyRow.js | 2 +- components/lib/galleria/Galleria.js | 4 +- components/lib/galleria/GalleriaThumbnails.js | 7 +- components/lib/hooks/useMergeProps.js | 8 +- components/lib/passthrough/index.d.ts | 12 ++ components/lib/treetable/TreeTableRow.js | 14 +-- components/lib/utils/MergeProps.js | 116 +++++++----------- components/lib/utils/Utils.js | 4 +- components/lib/utils/utils.d.ts | 13 +- 12 files changed, 95 insertions(+), 102 deletions(-) diff --git a/components/lib/carousel/Carousel.js b/components/lib/carousel/Carousel.js index 30ad3aae09..00999dcc75 100644 --- a/components/lib/carousel/Carousel.js +++ b/components/lib/carousel/Carousel.js @@ -1,18 +1,17 @@ import * as React from 'react'; import PrimeReact, { PrimeReactContext, localeOption } from '../api/Api'; import { useHandleStyle } from '../componentbase/ComponentBase'; -import { useMountEffect, usePrevious, useResizeListener, useUpdateEffect } from '../hooks/Hooks'; +import { useMergeProps, useMountEffect, usePrevious, useResizeListener, useUpdateEffect } from '../hooks/Hooks'; import { ChevronDownIcon } from '../icons/chevrondown'; import { ChevronLeftIcon } from '../icons/chevronleft'; import { ChevronRightIcon } from '../icons/chevronright'; import { ChevronUpIcon } from '../icons/chevronup'; import { Ripple } from '../ripple/Ripple'; -import { mergeProps } from '../utils/Utils'; - import { DomHandler, IconUtils, ObjectUtils, UniqueComponentId, classNames } from '../utils/Utils'; import { CarouselBase } from './CarouselBase'; const CarouselItem = React.memo((props) => { + const mergeProps = useMergeProps(); const { ptm, cx } = props; const key = props.className && props.className === 'p-carousel-item-cloned' ? 'itemCloned' : 'item'; const content = props.template(props.item); @@ -36,6 +35,7 @@ const CarouselItem = React.memo((props) => { export const Carousel = React.memo( React.forwardRef((inProps, ref) => { + const mergeProps = useMergeProps(); const context = React.useContext(PrimeReactContext); const props = CarouselBase.getProps(inProps, context); diff --git a/components/lib/colorpicker/ColorPickerPanel.js b/components/lib/colorpicker/ColorPickerPanel.js index c8b9edd08a..f320e6eddd 100644 --- a/components/lib/colorpicker/ColorPickerPanel.js +++ b/components/lib/colorpicker/ColorPickerPanel.js @@ -1,10 +1,11 @@ import * as React from 'react'; import { PrimeReactContext } from '../api/Api'; import { CSSTransition } from '../csstransition/CSSTransition'; +import { useMergeProps } from '../hooks/Hooks'; import { Portal } from '../portal/Portal'; -import { mergeProps } from '../utils/Utils'; export const ColorPickerPanel = React.forwardRef((props, ref) => { + const mergeProps = useMergeProps(); const context = React.useContext(PrimeReactContext); const { ptm, cx } = props; diff --git a/components/lib/componentbase/ComponentBase.js b/components/lib/componentbase/ComponentBase.js index f76533d689..58b07cec28 100644 --- a/components/lib/componentbase/ComponentBase.js +++ b/components/lib/componentbase/ComponentBase.js @@ -1,6 +1,6 @@ import PrimeReact from '../api/Api'; import { useMountEffect, useStyle, useUnmountEffect, useUpdateEffect } from '../hooks/Hooks'; -import { ObjectUtils, _mergeProps, classNames } from '../utils/Utils'; +import { ObjectUtils, classNames, mergeProps } from '../utils/Utils'; const baseStyle = ` .p-hidden-accessible { @@ -539,7 +539,7 @@ export const ComponentBase = { return mergeSections || (!mergeSections && self) ? useMergeProps - ? _mergeProps([globalPT, self, Object.keys(datasetProps).length ? datasetProps : {}], { classNameMergeFunction: ComponentBase.context.ptOptions?.classNameMergeFunction }) + ? mergeProps([globalPT, self, Object.keys(datasetProps).length ? datasetProps : {}], { classNameMergeFunction: ComponentBase.context.ptOptions?.classNameMergeFunction }) : { ...globalPT, ...self, ...(Object.keys(datasetProps).length ? datasetProps : {}) } : { ...self, ...(Object.keys(datasetProps).length ? datasetProps : {}) }; }; @@ -562,7 +562,7 @@ export const ComponentBase = { const self = getOptionValue(css && css.inlineStyles, key, { props, state, ...params }); const base = getOptionValue(inlineStyles, key, { props, state, ...params }); - return _mergeProps([base, self], { classNameMergeFunction: ComponentBase.context.ptOptions?.classNameMergeFunction }); + return mergeProps([base, self], { classNameMergeFunction: ComponentBase.context.ptOptions?.classNameMergeFunction }); } return undefined; @@ -620,7 +620,7 @@ const _usePT = (pt, callback, key, params) => { else if (ObjectUtils.isString(value)) return value; else if (ObjectUtils.isString(originalValue)) return originalValue; - return mergeSections || (!mergeSections && value) ? (useMergeProps ? _mergeProps([originalValue, value], { classNameMergeFunction }) : { ...originalValue, ...value }) : value; + return mergeSections || (!mergeSections && value) ? (useMergeProps ? mergeProps([originalValue, value], { classNameMergeFunction }) : { ...originalValue, ...value }) : value; } return fn(pt); diff --git a/components/lib/datatable/BodyRow.js b/components/lib/datatable/BodyRow.js index 10b380097f..670b92af21 100644 --- a/components/lib/datatable/BodyRow.js +++ b/components/lib/datatable/BodyRow.js @@ -493,7 +493,7 @@ export const BodyRow = React.memo((props) => { }, getBodyRowPTOptions('bodyRow'), { - className: classNames(rowClassName) + className: rowClassName // #5983 must be last so all unstyled merging takes place first } ); diff --git a/components/lib/galleria/Galleria.js b/components/lib/galleria/Galleria.js index 9288f4182c..1a0a50abc5 100644 --- a/components/lib/galleria/Galleria.js +++ b/components/lib/galleria/Galleria.js @@ -2,8 +2,7 @@ import * as React from 'react'; import PrimeReact, { PrimeReactContext, localeOption } from '../api/Api'; import { useHandleStyle } from '../componentbase/ComponentBase'; import { CSSTransition } from '../csstransition/CSSTransition'; -import { useInterval, useUnmountEffect } from '../hooks/Hooks'; -import { mergeProps } from '../utils/Utils'; +import { useInterval, useMergeProps, useUnmountEffect } from '../hooks/Hooks'; import { TimesIcon } from '../icons/times'; import { Portal } from '../portal/Portal'; import { Ripple } from '../ripple/Ripple'; @@ -14,6 +13,7 @@ import { GalleriaThumbnails } from './GalleriaThumbnails'; export const Galleria = React.memo( React.forwardRef((inProps, ref) => { + const mergeProps = useMergeProps(); const context = React.useContext(PrimeReactContext); const props = GalleriaBase.getProps(inProps, context); diff --git a/components/lib/galleria/GalleriaThumbnails.js b/components/lib/galleria/GalleriaThumbnails.js index bfa553355c..ce857f9c3f 100644 --- a/components/lib/galleria/GalleriaThumbnails.js +++ b/components/lib/galleria/GalleriaThumbnails.js @@ -1,15 +1,15 @@ import * as React from 'react'; import PrimeReact, { PrimeReactContext, localeOption } from '../api/Api'; -import { useMountEffect, usePrevious, useResizeListener, useUpdateEffect } from '../hooks/Hooks'; +import { useMergeProps, useMountEffect, usePrevious, useResizeListener, useUpdateEffect } from '../hooks/Hooks'; import { ChevronDownIcon } from '../icons/chevrondown'; import { ChevronLeftIcon } from '../icons/chevronleft'; import { ChevronRightIcon } from '../icons/chevronright'; import { ChevronUpIcon } from '../icons/chevronup'; - import { Ripple } from '../ripple/Ripple'; import { DomHandler, IconUtils, ObjectUtils, UniqueComponentId, classNames } from '../utils/Utils'; -import { mergeProps } from '../utils/Utils'; + const GalleriaThumbnailItem = React.memo((props) => { + const mergeProps = useMergeProps(); const { ptm, cx } = props; const getPTOptions = (key, options) => { @@ -165,6 +165,7 @@ const GalleriaThumbnailItem = React.memo((props) => { export const GalleriaThumbnails = React.memo( React.forwardRef((props, ref) => { + const mergeProps = useMergeProps(); const [numVisibleState, setNumVisibleState] = React.useState(props.numVisible); const [totalShiftedItemsState, setTotalShiftedItemsState] = React.useState(0); const itemsContainerRef = React.useRef(null); diff --git a/components/lib/hooks/useMergeProps.js b/components/lib/hooks/useMergeProps.js index 670dcffa7d..c49f195eea 100644 --- a/components/lib/hooks/useMergeProps.js +++ b/components/lib/hooks/useMergeProps.js @@ -1,6 +1,6 @@ import { useContext } from 'react'; import { PrimeReactContext } from '../api/Api'; -import { _mergeProps } from '../utils/Utils'; +import { mergeProps } from '../utils/Utils'; /** * Hook to merge properties including custom merge function for things like Tailwind merge. @@ -9,10 +9,6 @@ export const useMergeProps = () => { const context = useContext(PrimeReactContext); return (...props) => { - const options = { - classNameMergeFunction: context?.ptOptions?.classNameMergeFunction - }; - - return _mergeProps(props, options); + return mergeProps(props, context?.ptOptions); }; }; diff --git a/components/lib/passthrough/index.d.ts b/components/lib/passthrough/index.d.ts index 92cffa3794..09b7b9e77b 100644 --- a/components/lib/passthrough/index.d.ts +++ b/components/lib/passthrough/index.d.ts @@ -1,6 +1,18 @@ export interface PassThroughOptions { + /** + * Should the passthrough merge sections? + */ mergeSections?: boolean | undefined; + /** + * Should the passthrough merge properties? + */ mergeProps?: boolean | undefined; + /** + * Custom merge function such as twMerge for Tailwind merging. + * @param className1 the first className to merge + * @param className2 the second className to merge to className1 + * @returns the merged className + */ classNameMergeFunction?: (className1: string, className2: string) => string | undefined; } diff --git a/components/lib/treetable/TreeTableRow.js b/components/lib/treetable/TreeTableRow.js index c6029bbf95..cf95a35ca4 100644 --- a/components/lib/treetable/TreeTableRow.js +++ b/components/lib/treetable/TreeTableRow.js @@ -667,19 +667,16 @@ export const TreeTableRow = React.memo((props) => { const cells = props.columns.map(createCell); const children = createChildren(); - let className = cx('row', { isSelected, rowProps: props }); + let rowClassName = null; if (props.rowClassName) { - let rowClassName = props.rowClassName(props.node); - - className = { ...className, ...rowClassName }; + rowClassName = props.rowClassName(props.node); } - className = classNames(className, props.node.className); const rowProps = mergeProps( { tabIndex: 0, - className, + className: classNames(cx('row', { isSelected, rowProps: props })), 'aria-expanded': expanded, 'aria-level': props.level + 1, 'aria-posinset': props.ariaPosInSet, @@ -695,7 +692,10 @@ export const TreeTableRow = React.memo((props) => { onMouseLeave: (e) => onMouseLeave(e), 'data-p-highlight': isSelected() }, - getRowPTOptions('row') + getRowPTOptions('row'), + { + className: classNames(rowClassName, props.node.className) // #5983 must be last so all unstyled merging takes place first + } ); return ( diff --git a/components/lib/utils/MergeProps.js b/components/lib/utils/MergeProps.js index 28f4d60a59..5b1e868c04 100644 --- a/components/lib/utils/MergeProps.js +++ b/components/lib/utils/MergeProps.js @@ -1,77 +1,51 @@ -export function _mergeProps(props, options = {}) { - if (props) { - const { classNameMergeFunction } = options; - const isFn = (o) => !!(o && o.constructor && o.call && o.apply); - - return props.reduce((merged, ps) => { - for (const key in ps) { - const value = ps[key]; - - if (key === 'style') { - merged['style'] = { ...merged['style'], ...ps['style'] }; - } else if (key === 'className') { - let newClassname = ''; - - if (classNameMergeFunction && classNameMergeFunction instanceof Function) { - newClassname = classNameMergeFunction(merged['className'], ps['className']); - } else { - newClassname = [merged['className'], ps['className']].join(' ').trim(); - } - - const isEmpty = newClassname === null || newClassname === undefined || newClassname === ''; - - merged['className'] = isEmpty ? undefined : newClassname; - } else if (isFn(value)) { - const fn = merged[key]; - - merged[key] = fn - ? (...args) => { - fn(...args); - value(...args); - } - : value; +/** + * Merges properties together taking an Array of props and merging into one single set of + * properties. The options can contain a "classNameMergeFunction" which can be something + * like Tailwind Merge for properly merging Tailwind classes. + * + * @param {object[]} props the array of object properties to merge + * @param {*} options either empty or could contain a custom merge function like TailwindMerge + * @returns the single properties value after merging + */ +export function mergeProps(props, options = {}) { + if (!props) return undefined; + + const isFunction = (obj) => typeof obj === 'function'; + const { classNameMergeFunction } = options; + const hasMergeFunction = isFunction(classNameMergeFunction); + + return props.reduce((merged, ps) => { + if (!ps) return merged; + + for (const key in ps) { + const value = ps[key]; + + if (key === 'style') { + merged['style'] = { ...merged['style'], ...ps['style'] }; + } else if (key === 'className') { + let newClassName = ''; + + if (hasMergeFunction) { + newClassName = classNameMergeFunction(merged['className'], ps['className']); } else { - merged[key] = value; + newClassName = [merged['className'], ps['className']].join(' ').trim(); } - } - - return merged; - }, {}); - } - return undefined; -} - -// @todo - Update this function and remove _mergeProps -export function mergeProps(...props) { - if (props) { - const isFn = (o) => !!(o && o.constructor && o.call && o.apply); - - return props.reduce((merged, ps) => { - for (const key in ps) { - const value = ps[key]; - - if (key === 'style') { - merged['style'] = { ...merged['style'], ...ps['style'] }; - } else if (key === 'className') { - merged['className'] = [merged['className'], ps['className']].join(' ').trim(); - } else if (isFn(value)) { - const fn = merged[key]; - - merged[key] = fn - ? (...args) => { - fn(...args); - value(...args); - } - : value; - } else { - merged[key] = value; - } + merged['className'] = newClassName || undefined; + } else if (isFunction(value)) { + const existingFn = merged[key]; + + merged[key] = existingFn + ? (...args) => { + existingFn(...args); + value(...args); + } + : value; + } else { + merged[key] = value; } + } - return merged; - }, {}); - } - - return undefined; + return merged; + }, {}); } diff --git a/components/lib/utils/Utils.js b/components/lib/utils/Utils.js index 5526a3b9cf..1870e37861 100644 --- a/components/lib/utils/Utils.js +++ b/components/lib/utils/Utils.js @@ -3,9 +3,9 @@ import DomHandler from './DomHandler'; import EventBus from './EventBus'; import IconUtils from './IconUtils'; import { mask } from './Mask'; -import { _mergeProps, mergeProps } from './MergeProps'; +import { mergeProps } from './MergeProps'; import ObjectUtils from './ObjectUtils'; import UniqueComponentId from './UniqueComponentId'; import { ZIndexUtils } from './ZIndexUtils'; -export { DomHandler, EventBus, IconUtils, ObjectUtils, UniqueComponentId, ZIndexUtils, _mergeProps, classNames, mask, mergeProps }; +export { DomHandler, EventBus, IconUtils, ObjectUtils, UniqueComponentId, ZIndexUtils, classNames, mask, mergeProps }; diff --git a/components/lib/utils/utils.d.ts b/components/lib/utils/utils.d.ts index 9081271961..01e5378692 100644 --- a/components/lib/utils/utils.d.ts +++ b/components/lib/utils/utils.d.ts @@ -6,11 +6,20 @@ * */ import * as React from 'react'; +import { PassThroughOptions } from '../passthrough'; export declare function classNames(...args: any[]): string | undefined; -// @todo - replace it with mergeProps -export declare function _mergeProps(args: object[], options?: any): object | undefined; +/** + * Merges properties together taking an Array of props and merging into one single set of + * properties. The options can contain a "classNameMergeFunction" which can be something + * like Tailwind Merge for properly merging Tailwind classes. + * + * @param {object[]} args the array of object properties to merge + * @param {PassThroughOptions} options either empty or could contain a custom merge function like TailwindMerge + * @returns the single properties value after merging + */ +export declare function mergeProps(args: object[], options?: PassThroughOptions): object | undefined; export declare class DomHandler { static innerWidth(el: HTMLElement): number;