From c6723da7808062db511b051780269784fce01b0b Mon Sep 17 00:00:00 2001 From: Matiss Janis Aboltins <matiss@mja.lv> Date: Fri, 22 Dec 2023 21:20:29 +0000 Subject: [PATCH] :recycle: (TypeScript) fix strictFunctionTypes violations (pt 4) (#2071) --- .../src/components/common/Menu.tsx | 10 +- .../src/components/payees/PayeeTable.tsx | 99 ++-- .../components/schedules/SchedulesTable.tsx | 2 +- .../desktop-client/src/components/table.tsx | 498 +++++++++--------- .../transactions/TransactionsTable.jsx | 2 +- .../src/server/sync/sync.property.test.ts | 2 +- upcoming-release-notes/2071.md | 6 + 7 files changed, 309 insertions(+), 310 deletions(-) create mode 100644 upcoming-release-notes/2071.md diff --git a/packages/desktop-client/src/components/common/Menu.tsx b/packages/desktop-client/src/components/common/Menu.tsx index 1d0258cd7..eb5efa66c 100644 --- a/packages/desktop-client/src/components/common/Menu.tsx +++ b/packages/desktop-client/src/components/common/Menu.tsx @@ -33,19 +33,19 @@ type MenuItem = { key?: string; }; -type MenuProps = { +type MenuProps<T extends MenuItem = MenuItem> = { header?: ReactNode; footer?: ReactNode; - items: Array<MenuItem | typeof Menu.line>; - onMenuSelect: (itemName: MenuItem['name']) => void; + items: Array<T | typeof Menu.line>; + onMenuSelect: (itemName: T['name']) => void; }; -export default function Menu({ +export default function Menu<T extends MenuItem>({ header, footer, items: allItems, onMenuSelect, -}: MenuProps) { +}: MenuProps<T>) { const elRef = useRef(null); const items = allItems.filter(x => x); const [hoveredIndex, setHoveredIndex] = useState(null); diff --git a/packages/desktop-client/src/components/payees/PayeeTable.tsx b/packages/desktop-client/src/components/payees/PayeeTable.tsx index b376752ae..c7504d76c 100644 --- a/packages/desktop-client/src/components/payees/PayeeTable.tsx +++ b/packages/desktop-client/src/components/payees/PayeeTable.tsx @@ -1,10 +1,8 @@ import { - forwardRef, useCallback, useLayoutEffect, useState, type ComponentProps, - type ComponentRef, } from 'react'; import { type PayeeEntity } from 'loot-core/src/types/models'; @@ -20,6 +18,7 @@ import PayeeTableRow from './PayeeTableRow'; type PayeeWithId = PayeeEntity & Required<Pick<PayeeEntity, 'id'>>; type PayeeTableProps = { + tableRef: ComponentProps<typeof Table<PayeeWithId>>['tableRef']; payees: PayeeWithId[]; ruleCounts: Map<PayeeWithId['id'], number>; navigator: TableNavigator<PayeeWithId>; @@ -28,56 +27,56 @@ type PayeeTableProps = { 'onUpdate' | 'onViewRules' | 'onCreateRule' >; -const PayeeTable = forwardRef< - ComponentRef<typeof Table<PayeeWithId>>, - PayeeTableProps ->( - ( - { payees, ruleCounts, navigator, onUpdate, onViewRules, onCreateRule }, - ref, - ) => { - const [hovered, setHovered] = useState(null); - const selectedItems = useSelectedItems(); +const PayeeTable = ({ + tableRef, + payees, + ruleCounts, + navigator, + onUpdate, + onViewRules, + onCreateRule, +}: PayeeTableProps) => { + const [hovered, setHovered] = useState(null); + const selectedItems = useSelectedItems(); - useLayoutEffect(() => { - const firstSelected = [...selectedItems][0] as string; - if (typeof ref !== 'function') { - ref.current.scrollTo(firstSelected, 'center'); - } - navigator.onEdit(firstSelected, 'select'); - }, []); + useLayoutEffect(() => { + const firstSelected = [...selectedItems][0] as string; + if (typeof tableRef !== 'function') { + tableRef.current.scrollTo(firstSelected, 'center'); + } + navigator.onEdit(firstSelected, 'select'); + }, []); - const onHover = useCallback(id => { - setHovered(id); - }, []); + const onHover = useCallback((id: string) => { + setHovered(id); + }, []); - return ( - <View style={{ flex: 1 }} onMouseLeave={() => setHovered(null)}> - <Table<PayeeWithId> - ref={ref} - items={payees} - navigator={navigator} - renderItem={({ item, editing, focusedField, onEdit }) => { - return ( - <PayeeTableRow - payee={item} - ruleCount={ruleCounts.get(item.id) || 0} - selected={selectedItems.has(item.id)} - editing={editing} - focusedField={focusedField} - hovered={hovered === item.id} - onHover={onHover} - onEdit={onEdit} - onUpdate={onUpdate} - onViewRules={onViewRules} - onCreateRule={onCreateRule} - /> - ); - }} - /> - </View> - ); - }, -); + return ( + <View style={{ flex: 1 }} onMouseLeave={() => setHovered(null)}> + <Table + tableRef={tableRef} + items={payees} + navigator={navigator} + renderItem={({ item, editing, focusedField, onEdit }) => { + return ( + <PayeeTableRow + payee={item} + ruleCount={ruleCounts.get(item.id) || 0} + selected={selectedItems.has(item.id)} + editing={editing} + focusedField={focusedField} + hovered={hovered === item.id} + onHover={onHover} + onEdit={onEdit} + onUpdate={onUpdate} + onViewRules={onViewRules} + onCreateRule={onCreateRule} + /> + ); + }} + /> + </View> + ); +}; export default PayeeTable; diff --git a/packages/desktop-client/src/components/schedules/SchedulesTable.tsx b/packages/desktop-client/src/components/schedules/SchedulesTable.tsx index 149cb6fd1..e7c257493 100644 --- a/packages/desktop-client/src/components/schedules/SchedulesTable.tsx +++ b/packages/desktop-client/src/components/schedules/SchedulesTable.tsx @@ -118,7 +118,7 @@ function OverflowMenu({ onClose={() => setOpen(false)} > <Menu - onMenuSelect={(name: ScheduleItemAction) => { + onMenuSelect={name => { onAction(name, schedule.id); setOpen(false); }} diff --git a/packages/desktop-client/src/components/table.tsx b/packages/desktop-client/src/components/table.tsx index 92e2e833a..ddf274b1b 100644 --- a/packages/desktop-client/src/components/table.tsx +++ b/packages/desktop-client/src/components/table.tsx @@ -11,7 +11,6 @@ import React, { type ReactNode, type KeyboardEvent, type UIEvent, - type ReactElement, type Ref, } from 'react'; import { useStore } from 'react-redux'; @@ -847,17 +846,18 @@ type TableHandleRef<T extends TableItem = TableItem> = { type TableWithNavigatorProps = TableProps & { fields; }; -export const TableWithNavigator = forwardRef< - TableHandleRef<TableItem>, - TableWithNavigatorProps ->(({ fields, ...props }, ref) => { +export const TableWithNavigator = ({ + fields, + ...props +}: TableWithNavigatorProps) => { const navigator = useTableNavigator(props.items, fields); return <Table {...props} navigator={navigator} />; -}); +}; type TableItem = { id: number | string }; type TableProps<T extends TableItem = TableItem> = { + tableRef?: Ref<TableHandleRef<T>>; items: T[]; count?: number; headers?: ReactNode | TableHeaderProps['headers']; @@ -886,282 +886,276 @@ type TableProps<T extends TableItem = TableItem> = { saveScrollWidth?: (parent, child) => void; }; -export const Table: <T extends TableItem>( - props: TableProps<T> & { ref?: Ref<TableHandleRef<T>> }, -) => ReactElement = forwardRef<TableHandleRef, TableProps>( - ( - { - items, - count, - headers, - contentHeader, - loading, - rowHeight = ROW_HEIGHT, - backgroundColor = theme.tableHeaderBackground, - renderItem, - renderEmpty, - getItemKey, - loadMore, - style, - navigator, - onScroll, - version = 'v1', - allowPopupsEscape, - isSelected, - saveScrollWidth, - ...props - }, - ref, - ) => { - if (!navigator) { - navigator = { - onEdit: () => {}, - editingId: null, - focusedField: null, - getNavigatorProps: props => props, - }; - } - - const { onEdit, editingId, focusedField, getNavigatorProps } = navigator; - const list = useRef(null); - const listContainer = useRef(null); - const scrollContainer = useRef(null); - const initialScrollTo = useRef(null); - const listInitialized = useRef(false); - - useImperativeHandle(ref, () => ({ - scrollTo: (id, alignment = 'smart') => { - const index = items.findIndex(item => item.id === id); - if (index !== -1) { - if (!list.current) { - // If the table hasn't been laid out yet, we need to wait for - // that to happen before we can scroll to something - initialScrollTo.current = index; - } else { - list.current.scrollToItem(index, alignment); - } - } - }, - - scrollToTop: () => { - list.current?.scrollTo(0); - }, +export const Table = <T extends TableItem>({ + tableRef, + items, + count, + headers, + contentHeader, + loading, + rowHeight = ROW_HEIGHT, + backgroundColor = theme.tableHeaderBackground, + renderItem, + renderEmpty, + getItemKey, + loadMore, + style, + navigator, + onScroll, + version = 'v1', + allowPopupsEscape, + isSelected, + saveScrollWidth, + ...props +}: TableProps<T>) => { + if (!navigator) { + navigator = { + onEdit: () => {}, + editingId: null, + focusedField: null, + getNavigatorProps: props => props, + }; + } - getScrolledItem: () => { - if (scrollContainer.current) { - const offset = scrollContainer.current.scrollTop; - const index = list.current.getStartIndexForOffset(offset); - return items[index].id; + const { onEdit, editingId, focusedField, getNavigatorProps } = navigator; + const list = useRef(null); + const listContainer = useRef(null); + const scrollContainer = useRef(null); + const initialScrollTo = useRef(null); + const listInitialized = useRef(false); + + useImperativeHandle(tableRef, () => ({ + scrollTo: (id, alignment = 'smart') => { + const index = items.findIndex(item => item.id === id); + if (index !== -1) { + if (!list.current) { + // If the table hasn't been laid out yet, we need to wait for + // that to happen before we can scroll to something + initialScrollTo.current = index; + } else { + list.current.scrollToItem(index, alignment); } - return 0; - }, - - setRowAnimation: flag => { - list.current?.setRowAnimation(flag); - }, + } + }, - edit(id, field, shouldScroll) { - onEdit(id, field); + scrollToTop: () => { + list.current?.scrollTo(0); + }, - if (id && shouldScroll) { - // @ts-expect-error this should not be possible - ref.scrollTo(id); - } - }, + getScrolledItem: () => { + if (scrollContainer.current) { + const offset = scrollContainer.current.scrollTop; + const index = list.current.getStartIndexForOffset(offset); + return items[index].id; + } + return 0; + }, - anchor() { - list.current?.anchor(); - }, + setRowAnimation: flag => { + list.current?.setRowAnimation(flag); + }, - unanchor() { - list.current?.unanchor(); - }, + edit(id, field, shouldScroll) { + onEdit(id, field); - isAnchored() { - return list.current && list.current.isAnchored(); - }, - })); - - useLayoutEffect(() => { - // We wait for the list to mount because AutoSizer needs to run - // before it's mounted - if (!listInitialized.current && listContainer.current) { - // Animation is on by default - list.current?.setRowAnimation(true); - listInitialized.current = true; + if (id && shouldScroll) { + // @ts-expect-error this should not be possible + ref.scrollTo(id); } + }, - if (scrollContainer.current && saveScrollWidth) { - saveScrollWidth( - scrollContainer.current.offsetParent - ? scrollContainer.current.offsetParent.clientWidth - : 0, - scrollContainer.current ? scrollContainer.current.clientWidth : 0, - ); - } - }); + anchor() { + list.current?.anchor(); + }, - function renderRow({ index, style, key }) { - const item = items[index]; - const editing = editingId === item.id; - const selected = isSelected && isSelected(item.id); - - const row = renderItem({ - item, - editing, - focusedField: editing && focusedField, - onEdit, - index, - position: style.top, - }); - - // TODO: Need to also apply zIndex if item is selected - // * Port over ListAnimation to Table - // * Move highlighted functionality into here - return ( - <View - key={key} - style={{ - ...rowStyle, - zIndex: editing || selected ? 101 : 'auto', - transform: 'translateY(var(--pos))', - }} - // @ts-expect-error not a recognised style attribute - nativeStyle={{ '--pos': `${style.top - 1}px` }} - data-focus-key={item.id} - > - {row} - </View> - ); + unanchor() { + list.current?.unanchor(); + }, + + isAnchored() { + return list.current && list.current.isAnchored(); + }, + })); + + useLayoutEffect(() => { + // We wait for the list to mount because AutoSizer needs to run + // before it's mounted + if (!listInitialized.current && listContainer.current) { + // Animation is on by default + list.current?.setRowAnimation(true); + listInitialized.current = true; } - function getScrollOffset(height, index) { - return ( - index * (rowHeight - 1) + - (rowHeight - 1) / 2 - - height / 2 + - (rowHeight - 1) * 2 + if (scrollContainer.current && saveScrollWidth) { + saveScrollWidth( + scrollContainer.current.offsetParent + ? scrollContainer.current.offsetParent.clientWidth + : 0, + scrollContainer.current ? scrollContainer.current.clientWidth : 0, ); } + }); - function onItemsRendered({ overscanStartIndex, overscanStopIndex }) { - if (loadMore && overscanStopIndex > items.length - 100) { - loadMore(); - } - } + function renderRow({ index, style, key }) { + const item = items[index]; + const editing = editingId === item.id; + const selected = isSelected && isSelected(item.id); - function getEmptyContent(empty) { - if (empty == null) { - return null; - } else if (typeof empty === 'function') { - return empty(); - } + const row = renderItem({ + item, + editing, + focusedField: editing && focusedField, + onEdit, + index, + position: style.top, + }); - return ( - <View - style={{ - justifyContent: 'center', - alignItems: 'center', - fontStyle: 'italic', - color: theme.tableText, - flex: 1, - }} - > - {empty} - </View> - ); - } + // TODO: Need to also apply zIndex if item is selected + // * Port over ListAnimation to Table + // * Move highlighted functionality into here + return ( + <View + key={key} + style={{ + ...rowStyle, + zIndex: editing || selected ? 101 : 'auto', + transform: 'translateY(var(--pos))', + }} + // @ts-expect-error not a recognised style attribute + nativeStyle={{ '--pos': `${style.top - 1}px` }} + data-focus-key={item.id} + > + {row} + </View> + ); + } - if (loading) { - return ( - <View - style={{ - flex: 1, - justifyContent: 'center', - alignItems: 'center', - backgroundColor, - }} - > - <AnimatedLoading width={25} color={theme.tableText} /> - </View> - ); + function getScrollOffset(height, index) { + return ( + index * (rowHeight - 1) + + (rowHeight - 1) / 2 - + height / 2 + + (rowHeight - 1) * 2 + ); + } + + function onItemsRendered({ overscanStartIndex, overscanStopIndex }) { + if (loadMore && overscanStopIndex > items.length - 100) { + loadMore(); } + } - const isEmpty = (count || items.length) === 0; + function getEmptyContent(empty) { + if (empty == null) { + return null; + } else if (typeof empty === 'function') { + return empty(); + } return ( <View style={{ + justifyContent: 'center', + alignItems: 'center', + fontStyle: 'italic', + color: theme.tableText, flex: 1, - outline: 'none', - ...style, }} - tabIndex={1} - {...getNavigatorProps(props)} - data-testid="table" > - {headers && ( - <TableHeader - height={rowHeight} - {...(Array.isArray(headers) ? { headers } : { children: headers })} - /> - )} - <View - style={{ - flex: `1 1 ${rowHeight * Math.max(2, items.length)}px`, - backgroundColor, - }} - > - {isEmpty ? ( - getEmptyContent(renderEmpty) - ) : ( - <AutoSizer> - {({ width, height }) => { - if (width === 0 || height === 0) { - return null; - } + {empty} + </View> + ); + } - return ( - <IntersectionBoundary.Provider - value={!allowPopupsEscape && listContainer} - > - <AvoidRefocusScrollProvider> - <FixedSizeList - ref={list} - header={contentHeader} - innerRef={listContainer} - outerRef={scrollContainer} - width={width} - height={height} - renderRow={renderRow} - itemCount={count || items.length} - itemSize={rowHeight - 1} - itemKey={ - getItemKey || ((index: number) => items[index].id) - } - indexForKey={key => - items.findIndex(item => item.id === key) - } - initialScrollOffset={ - initialScrollTo.current - ? getScrollOffset(height, initialScrollTo.current) - : 0 - } - overscanCount={5} - onItemsRendered={onItemsRendered} - onScroll={onScroll} - /> - </AvoidRefocusScrollProvider> - </IntersectionBoundary.Provider> - ); - }} - </AutoSizer> - )} - </View> + if (loading) { + return ( + <View + style={{ + flex: 1, + justifyContent: 'center', + alignItems: 'center', + backgroundColor, + }} + > + <AnimatedLoading width={25} color={theme.tableText} /> </View> ); - }, -); + } + + const isEmpty = (count || items.length) === 0; + + return ( + <View + style={{ + flex: 1, + outline: 'none', + ...style, + }} + tabIndex={1} + {...getNavigatorProps(props)} + data-testid="table" + > + {headers && ( + <TableHeader + height={rowHeight} + {...(Array.isArray(headers) ? { headers } : { children: headers })} + /> + )} + <View + style={{ + flex: `1 1 ${rowHeight * Math.max(2, items.length)}px`, + backgroundColor, + }} + > + {isEmpty ? ( + getEmptyContent(renderEmpty) + ) : ( + <AutoSizer> + {({ width, height }) => { + if (width === 0 || height === 0) { + return null; + } + + return ( + <IntersectionBoundary.Provider + value={!allowPopupsEscape && listContainer} + > + <AvoidRefocusScrollProvider> + <FixedSizeList + ref={list} + header={contentHeader} + innerRef={listContainer} + outerRef={scrollContainer} + width={width} + height={height} + renderRow={renderRow} + itemCount={count || items.length} + itemSize={rowHeight - 1} + itemKey={ + getItemKey || ((index: number) => items[index].id) + } + indexForKey={key => + items.findIndex(item => item.id === key) + } + initialScrollOffset={ + initialScrollTo.current + ? getScrollOffset(height, initialScrollTo.current) + : 0 + } + overscanCount={5} + onItemsRendered={onItemsRendered} + onScroll={onScroll} + /> + </AvoidRefocusScrollProvider> + </IntersectionBoundary.Provider> + ); + }} + </AutoSizer> + )} + </View> + </View> + ); +}; export type TableNavigator<T extends TableItem> = { onEdit: (id: T['id'], field?: string) => void; diff --git a/packages/desktop-client/src/components/transactions/TransactionsTable.jsx b/packages/desktop-client/src/components/transactions/TransactionsTable.jsx index a1009c1aa..5c04a9289 100644 --- a/packages/desktop-client/src/components/transactions/TransactionsTable.jsx +++ b/packages/desktop-client/src/components/transactions/TransactionsTable.jsx @@ -1722,7 +1722,7 @@ function TransactionTableInner({ > <Table navigator={tableNavigator} - ref={tableRef} + tableRef={tableRef} items={props.transactions} renderItem={renderRow} renderEmpty={renderEmpty} diff --git a/packages/loot-core/src/server/sync/sync.property.test.ts b/packages/loot-core/src/server/sync/sync.property.test.ts index 346eb3f16..d748f9d51 100644 --- a/packages/loot-core/src/server/sync/sync.property.test.ts +++ b/packages/loot-core/src/server/sync/sync.property.test.ts @@ -111,7 +111,7 @@ function makeGen<T extends Arbitrary<unknown>>({ value, timestamp: jsc.integer(1000, 10000).smap( x => { - let clientId; + let clientId: string; switch (jsc.random(0, 1)) { case 0: clientId = clientId1; diff --git a/upcoming-release-notes/2071.md b/upcoming-release-notes/2071.md new file mode 100644 index 000000000..e443cc99a --- /dev/null +++ b/upcoming-release-notes/2071.md @@ -0,0 +1,6 @@ +--- +category: Maintenance +authors: [MatissJanis] +--- + +Fixing TypeScript issues when enabling `strictFunctionTypes` (pt.4). -- GitLab