Skip to content

Commit

Permalink
Fix #5983: MergeProps refactor and optimize (#5996)
Browse files Browse the repository at this point in the history
  • Loading branch information
melloware authored Feb 18, 2024
1 parent a0567fb commit 511658c
Show file tree
Hide file tree
Showing 12 changed files with 95 additions and 102 deletions.
6 changes: 3 additions & 3 deletions components/lib/carousel/Carousel.js
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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);

Expand Down
3 changes: 2 additions & 1 deletion components/lib/colorpicker/ColorPickerPanel.js
Original file line number Diff line number Diff line change
@@ -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;

Expand Down
8 changes: 4 additions & 4 deletions components/lib/componentbase/ComponentBase.js
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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 : {}) };
};
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion components/lib/datatable/BodyRow.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
);

Expand Down
4 changes: 2 additions & 2 deletions components/lib/galleria/Galleria.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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);

Expand Down
7 changes: 4 additions & 3 deletions components/lib/galleria/GalleriaThumbnails.js
Original file line number Diff line number Diff line change
@@ -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) => {
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 2 additions & 6 deletions components/lib/hooks/useMergeProps.js
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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);
};
};
12 changes: 12 additions & 0 deletions components/lib/passthrough/index.d.ts
Original file line number Diff line number Diff line change
@@ -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;
}

Expand Down
14 changes: 7 additions & 7 deletions components/lib/treetable/TreeTableRow.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 (
Expand Down
116 changes: 45 additions & 71 deletions components/lib/utils/MergeProps.js
Original file line number Diff line number Diff line change
@@ -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;
}, {});
}
4 changes: 2 additions & 2 deletions components/lib/utils/Utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
13 changes: 11 additions & 2 deletions components/lib/utils/utils.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 511658c

Please sign in to comment.