Skip to content

Commit

Permalink
refactor: useEventNotStable type (#2741)
Browse files Browse the repository at this point in the history
This pull request includes several changes to improve type safety and optimize the performance of React components by using `useCallback` and ensuring proper type handling in hooks. The most important changes include modifications to `DynamicInputNumber`, `DynamicUnitInputNumberWithSlider`, and `VFolderTable` components, as well as updates to the `useEventNotStable` hook.

### Type Safety Improvements:
* [`react/src/components/DynamicStepInputNumber.tsx`](diffhunk://#diff-b3d890e46cddb08acfce125b27ff3da0ca9db076ab3ebcd35fadace06e7c8d92L32-R32): Updated the `updateKey` call to use `value.toString()` to ensure the value is a string.
* [`react/src/components/DynamicUnitInputNumberWithSlider.tsx`](diffhunk://#diff-26552acf4047ca1f6af64dbcf17ecdce04c84a7f669f055db97e13afe873a6e7L54-R54): Updated the `updateKey` call to use `value?.toString()` to handle potential undefined values safely.

### Performance Optimizations:
* [`react/src/components/VFolderTable.tsx`](diffhunk://#diff-14f34a27406ff418af22058ec3978654932614d41995f8f9a6f52f92e32f47c8L247-R263): Replaced `useEventNotStable` with `useCallback` for `inputToAliasPath` to improve performance and stability.
* [`react/src/hooks/useEventNotStable.tsx`](diffhunk://#diff-44cbecbfeea00712d81f6d06b52dbb0e5c7bad616c790244d4b238851e70e83dL3-R7): Enhanced the `useEventNotStable` hook to use generics for better type safety and updated the return type to `undefined` instead of `null`. [[1]](diffhunk://#diff-44cbecbfeea00712d81f6d06b52dbb0e5c7bad616c790244d4b238851e70e83dL3-R7) [[2]](diffhunk://#diff-44cbecbfeea00712d81f6d06b52dbb0e5c7bad616c790244d4b238851e70e83dL14-R19)

### Code Cleanup:
* [`react/src/components/VFolderTable.tsx`](diffhunk://#diff-14f34a27406ff418af22058ec3978654932614d41995f8f9a6f52f92e32f47c8L34-R40): Added missing imports for `useCallback` to ensure the code compiles correctly.
  • Loading branch information
yomybaby committed Oct 21, 2024
1 parent 805df28 commit 253d05e
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 10 deletions.
2 changes: 1 addition & 1 deletion react/src/components/DynamicStepInputNumber.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const DynamicInputNumber: React.FC<DynamicInputNumberProps> = ({
const [key, updateKey] = useUpdatableState('first');
useEffect(() => {
setTimeout(() => {
updateKey(value);
updateKey(value.toString());
}, 0);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
Expand Down
2 changes: 1 addition & 1 deletion react/src/components/DynamicUnitInputNumberWithSlider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const DynamicUnitInputNumberWithSlider: React.FC<
const [key, updateKey] = useUpdatableState('first');
useEffect(() => {
setTimeout(() => {
updateKey(value);
updateKey(value?.toString());
}, 0);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
Expand Down
15 changes: 11 additions & 4 deletions react/src/components/VFolderTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,13 @@ import { ColumnsType } from 'antd/lib/table';
import graphql from 'babel-plugin-relay/macro';
import dayjs from 'dayjs';
import _ from 'lodash';
import React, { useEffect, useMemo, useState, useTransition } from 'react';
import React, {
useCallback,
useEffect,
useMemo,
useState,
useTransition,
} from 'react';
import { Trans, useTranslation } from 'react-i18next';
import { useLazyLoadQuery } from 'react-relay';

Expand Down Expand Up @@ -244,16 +250,17 @@ const VFolderTable: React.FC<VFolderTableProps> = ({
* @param input - The input path to be converted.
* @returns The aliased path based on the name and input.
*/
const inputToAliasPath = useEventNotStable(
const inputToAliasPath = useCallback(
(name: VFolderKey, input?: string) => {
if (_.isEmpty(input)) {
if (input === undefined || input === '') {
return `${aliasBasePath}${name}`;
} else if (input?.startsWith('/')) {
} else if (input.startsWith('/')) {
return input;
} else {
return `${aliasBasePath}${input}`;
}
},
[aliasBasePath],
);

const handleAliasUpdate = useEventNotStable(() => {
Expand Down
10 changes: 6 additions & 4 deletions react/src/hooks/useEventNotStable.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { useCallback, useLayoutEffect, useRef } from 'react';

export function useEventNotStable(handler: (props: any) => void) {
export function useEventNotStable<Args extends unknown[], Return>(
handler: (...args: Args) => Return,
) {
// eslint-disable-next-line
const handlerRef = useRef<Function | null>(null);
const handlerRef = useRef<typeof handler | undefined>(undefined);

// In a real implementation, this would run before layout effects
useLayoutEffect(() => {
Expand All @@ -11,9 +13,9 @@ export function useEventNotStable(handler: (props: any) => void) {

// eslint-disable-next-line
// @ts-ignore
return useCallback((...args) => {
return useCallback((...args: Args) => {
// In a real implementation, this would throw if called during render
const fn = handlerRef.current;
return fn ? fn(...args) : null;
return fn ? fn(...args) : undefined;
}, []);
}

0 comments on commit 253d05e

Please sign in to comment.