diff --git a/.changeset/famous-students-sin.md b/.changeset/famous-students-sin.md new file mode 100644 index 0000000000..7911b8109e --- /dev/null +++ b/.changeset/famous-students-sin.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/fuselage": patch +--- + +Memory leak in `PaginatedMultiSelect` diff --git a/packages/fuselage/src/components/PaginatedSelect/PaginatedMultiSelect.tsx b/packages/fuselage/src/components/PaginatedSelect/PaginatedMultiSelect.tsx index 7304026725..28102febae 100644 --- a/packages/fuselage/src/components/PaginatedSelect/PaginatedMultiSelect.tsx +++ b/packages/fuselage/src/components/PaginatedSelect/PaginatedMultiSelect.tsx @@ -2,14 +2,12 @@ import { useMutableCallback, useResizeObserver, } from '@rocket.chat/fuselage-hooks'; -import type { - SyntheticEvent, - ComponentProps, - Ref, - ReactElement, - FormEvent, +import React, { + type ComponentProps, + type ReactElement, + useState, + useRef, } from 'react'; -import React, { useState, useRef, useCallback, forwardRef, memo } from 'react'; import { prevent } from '../../helpers/prevent'; import AnimatedVisibility from '../AnimatedVisibility'; @@ -17,7 +15,6 @@ import Box from '../Box'; import Chip from '../Chip'; import Flex from '../Flex'; import { Icon } from '../Icon'; -import { InputBox } from '../InputBox'; import Margins from '../Margins'; import { useVisible } from '../Options/useVisible'; import { OptionsPaginated } from '../OptionsPaginated'; @@ -25,10 +22,6 @@ import Position from '../Position'; import SelectAddon from '../Select/SelectAddon'; import SelectFocus from '../Select/SelectFocus'; -const SelectedOptions = memo>((props) => ( - -)); - export type PaginatedMultiSelectOption = { value: string | number; label: string; @@ -51,7 +44,7 @@ type PaginatedMultiSelectProps = Omit< anchor?: any; }; -export const PaginatedMultiSelect = ({ +const PaginatedMultiSelect = ({ withTitle, value, filter, @@ -61,7 +54,7 @@ export const PaginatedMultiSelect = ({ anchor: Anchor = SelectFocus, onChange = () => {}, placeholder, - renderOptions: _Options = OptionsPaginated, + renderOptions: OptionsComponent = OptionsPaginated, endReached, ...props }: PaginatedMultiSelectProps) => { @@ -73,19 +66,6 @@ export const PaginatedMultiSelect = ({ currentValue.some((opt) => opt.value === option.value) ); - const internalChanged = (option: PaginatedMultiSelectOption) => { - if (currentValue.some((opt) => opt.value === option.value)) { - const newValue = currentValue.filter((opt) => opt.value !== option.value); - - setInternalValue(newValue); - return onChange(newValue); - } - - const newValue = [...currentValue, option]; - setInternalValue(newValue); - return onChange(newValue); - }; - const [visible, hide, show] = useVisible(); const ref = useRef(null); @@ -101,13 +81,32 @@ export const PaginatedMultiSelect = ({ } }); - const handleMouseDown = useMutableCallback( - (option: PaginatedMultiSelectOption) => (e: SyntheticEvent) => { - prevent(e); - internalChanged(option); - return false; + const addOption = (value: unknown) => { + const option = options.find((opt) => opt.value === value); + if (!option) { + return; } - ); + + const newValue = [...currentValue, option]; + setInternalValue(newValue); + onChange(newValue); + }; + + const removeOption = (value: unknown) => { + const newValue = currentValue.filter((opt) => opt.value !== value); + + setInternalValue(newValue); + onChange(newValue); + }; + + const toggleOption = (value: unknown) => { + if (currentValue.some((opt) => opt.value === value)) { + removeOption(value); + return; + } + + addOption(value); + }; return ( - {selectedOptions.map((option, index: number) => ( - ( + + onClick={(e) => { + prevent(e); + removeOption(value); + }} + > + {label} + ))} @@ -179,8 +182,7 @@ export const PaginatedMultiSelect = ({ - <_Options - withTitle={withTitle} + - internalChanged({ value: value as string, label }) - } + onSelect={([value]) => { + toggleOption(value); + }} /> @@ -199,55 +201,4 @@ export const PaginatedMultiSelect = ({ ); }; -type PaginatedMultiSelectFilteredProps = Omit< - PaginatedMultiSelectProps, - 'filter' -> & { - filter?: string; - setFilter?: (value: string) => void; -}; - -export const PaginatedMultiSelectFiltered = ({ - filter, - setFilter, - options, - placeholder, - ...props -}: PaginatedMultiSelectFilteredProps) => { - // eslint-disable-next-line react-hooks/exhaustive-deps - const anchor = useCallback( - forwardRef( - ( - { - children: _children, - filter, - ...props - }: ComponentProps, - ref: Ref - ) => ( - - ) => - setFilter?.(e.currentTarget.value) - } - {...props} - rcx-input-box--undecorated - /> - - ) - ), - [] - ); - - return ( - - ); -}; +export default PaginatedMultiSelect; diff --git a/packages/fuselage/src/components/PaginatedSelect/PaginatedMultiSelectFiltered.stories.tsx b/packages/fuselage/src/components/PaginatedSelect/PaginatedMultiSelectFiltered.stories.tsx index 4b171d5cf5..2dce47e635 100644 --- a/packages/fuselage/src/components/PaginatedSelect/PaginatedMultiSelectFiltered.stories.tsx +++ b/packages/fuselage/src/components/PaginatedSelect/PaginatedMultiSelectFiltered.stories.tsx @@ -20,6 +20,10 @@ export default { }, } as ComponentMeta; +const Template: ComponentStory = ( + args +) => ; + export const Normal: ComponentStory = ( args ) => { @@ -33,16 +37,12 @@ export const Normal: ComponentStory = ( ); }; -export const Errored: ComponentStory = ( - args -) => ; +export const Errored = Template.bind({}); Errored.args = { error: true, }; -export const Disabled: ComponentStory = ( - args -) => ; +export const Disabled = Template.bind({}); Disabled.args = { disabled: true, }; diff --git a/packages/fuselage/src/components/PaginatedSelect/PaginatedMultiSelectFiltered.tsx b/packages/fuselage/src/components/PaginatedSelect/PaginatedMultiSelectFiltered.tsx new file mode 100644 index 0000000000..5503ee114f --- /dev/null +++ b/packages/fuselage/src/components/PaginatedSelect/PaginatedMultiSelectFiltered.tsx @@ -0,0 +1,74 @@ +import type { ComponentProps, Ref, FormEvent, ReactElement } from 'react'; +import React, { useCallback, forwardRef } from 'react'; + +import type Box from '../Box'; +import Flex from '../Flex'; +import { InputBox } from '../InputBox'; +import { type OptionsPaginated } from '../OptionsPaginated'; +import PaginatedMultiSelect, { + type PaginatedMultiSelectOption, +} from './PaginatedMultiSelect'; + +type PaginatedMultiSelectFilteredProps = Omit< + ComponentProps, + 'onChange' | 'value' | 'filter' +> & { + error?: boolean; + options: PaginatedMultiSelectOption[]; + withTitle?: boolean; + placeholder?: string; + endReached?: (start?: number, end?: number) => void; + value?: PaginatedMultiSelectOption[]; + onChange: (values: PaginatedMultiSelectOption[]) => void; + renderOptions?: ( + props: ComponentProps + ) => ReactElement | null; + anchor?: any; + filter?: string; + setFilter?: (value: string) => void; +}; + +export const PaginatedMultiSelectFiltered = ({ + filter, + setFilter, + options, + placeholder, + ...props +}: PaginatedMultiSelectFilteredProps) => { + // eslint-disable-next-line react-hooks/exhaustive-deps + const anchor = useCallback( + forwardRef( + ( + { + children: _children, + filter, + ...props + }: ComponentProps, + ref: Ref + ) => ( + + ) => + setFilter?.(e.currentTarget.value) + } + {...props} + rcx-input-box--undecorated + /> + + ) + ), + [] + ); + + return ( + + ); +}; diff --git a/packages/fuselage/src/components/PaginatedSelect/PaginatedSelect.tsx b/packages/fuselage/src/components/PaginatedSelect/PaginatedSelect.tsx index b40ad4e006..f077b0889e 100644 --- a/packages/fuselage/src/components/PaginatedSelect/PaginatedSelect.tsx +++ b/packages/fuselage/src/components/PaginatedSelect/PaginatedSelect.tsx @@ -1,12 +1,11 @@ -// @ts-nocheck import { useMutableCallback, useResizeObserver, } from '@rocket.chat/fuselage-hooks'; -import type { SyntheticEvent, ElementType } from 'react'; -import React, { useState, useRef, useMemo, useEffect } from 'react'; +import React, { type ElementType, useState, useRef, useMemo } from 'react'; -import type { SelectProps } from '..'; +import { type SelectProps } from '..'; +import { prevent } from '../../helpers/prevent'; import AnimatedVisibility from '../AnimatedVisibility'; import Box from '../Box'; import { Icon } from '../Icon'; @@ -30,24 +29,6 @@ export type PaginatedSelectProps = Omit & { setFilter?: (value: string | undefined | number) => void; }; -const prevent = (e: SyntheticEvent) => { - e.preventDefault(); - e.stopPropagation(); - e.nativeEvent.stopImmediatePropagation(); -}; - -const useDidUpdate = (func: string[]) => { - const didMount = useRef(false); - const fn = useMutableCallback(func as any); - - useEffect(() => { - if (didMount.current) { - fn(); - } - didMount.current = true; - }, [fn]); -}; - export const PaginatedSelect = ({ value, withTitle, @@ -81,8 +62,6 @@ export const PaginatedSelect = ({ const { ref: containerRef, borderBoxSize } = useResizeObserver(); - useDidUpdate([filter, internalValue]); - const valueLabel = option?.label; const visibleText = diff --git a/packages/fuselage/src/components/PaginatedSelect/PaginatedSelectFiltered.stories.tsx b/packages/fuselage/src/components/PaginatedSelect/PaginatedSelectFiltered.stories.tsx index 9fcf12a760..7b0c4e7362 100644 --- a/packages/fuselage/src/components/PaginatedSelect/PaginatedSelectFiltered.stories.tsx +++ b/packages/fuselage/src/components/PaginatedSelect/PaginatedSelectFiltered.stories.tsx @@ -18,20 +18,20 @@ export default { actions: { argTypesRegex: '^on.*' }, layout: 'centered', }, -} as ComponentMeta; +} satisfies ComponentMeta; const Template: ComponentStory = (args) => ( ); -export const normal = Template.bind({}); +export const Normal = Template.bind({}); -export const errored = Template.bind({}); -errored.args = { +export const Errored = Template.bind({}); +Errored.args = { error: 'Error', }; -export const disabled = Template.bind({}); -disabled.args = { +export const Disabled = Template.bind({}); +Disabled.args = { disabled: true, }; diff --git a/packages/fuselage/src/components/PaginatedSelect/index.ts b/packages/fuselage/src/components/PaginatedSelect/index.ts index 82db1fb635..935e70b7bf 100644 --- a/packages/fuselage/src/components/PaginatedSelect/index.ts +++ b/packages/fuselage/src/components/PaginatedSelect/index.ts @@ -1,5 +1,3 @@ export { PaginatedSelectFiltered } from './PaginatedSelectFiltered'; -export { - PaginatedMultiSelectFiltered, - PaginatedMultiSelectOption, -} from './PaginatedMultiSelect'; +export { PaginatedMultiSelectOption } from './PaginatedMultiSelect'; +export { PaginatedMultiSelectFiltered } from './PaginatedMultiSelectFiltered';