Skip to content

chore: revert ts type changes and use smaller way #51

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 3, 2025
Merged

chore: revert ts type changes and use smaller way #51

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a85163f
chore: revert ts type changes and use smaller way.
yoyo837 Sep 29, 2025
85fb58a
update
li-jia-nan Sep 29, 2025
fd65fbe
Apply suggestion from @gemini-code-assist[bot]
li-jia-nan Sep 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 14 additions & 19 deletions src/Immutable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ export type CompareProps<T extends React.ComponentType<any>> = (
nextProps: Readonly<React.ComponentProps<T>>,
) => boolean;

type ImmutableProps<T extends React.ComponentType<any>> = Omit<React.ComponentProps<T>, 'ref'>;

/**
* Create Immutable pair for `makeImmutable` and `responseImmutable`.
*/
Expand All @@ -34,24 +32,24 @@ export default function createImmutable() {
function makeImmutable<T extends React.ComponentType<any>>(
Component: T,
shouldTriggerRender?: CompareProps<T>,
): React.ComponentType<React.ComponentProps<T>> {
): T {
const refAble = supportRef(Component);

const ImmutableComponent = (props: ImmutableProps<T>, ref: React.Ref<any>) => {
const ImmutableComponent: React.ForwardRefRenderFunction<any, any> = (props, ref) => {
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

将 ImmutableComponent 的类型定义为 React.ForwardRefRenderFunction<any, any> 会导致 props 和 ref 失去类型信息，降低了类型安全性。虽然这可以避免在后续代码中使用 as any，但更好的做法是提供更精确的类型。

我建议使用 React.ElementRef<T> 和 React.ComponentPropsWithoutRef<T> 来精确地定义 ref 和 props 的类型。

请注意，这个改动可能会暴露出 shouldTriggerRender 函数的类型不匹配问题。它的类型 CompareProps<T> 期望的参数是 React.ComponentProps<T>，而 ImmutableComponent 的 props 类型是 React.ComponentPropsWithoutRef<T>。根本原因可能是 CompareProps<T> 的定义，它或许应该使用 React.ComponentPropsWithoutRef<T>。考虑到这是一个更深层次的问题，一个临时的解决方案可能是在调用 shouldTriggerRender 时进行类型断言，但这正是本次 PR 想要移除的。最好的方式是修复 CompareProps<T> 的类型定义。

Suggested change
const ImmutableComponent: React.ForwardRefRenderFunction<any, any> = (props, ref) => {
const ImmutableComponent: React.ForwardRefRenderFunction<
React.ElementRef<T>,
React.ComponentPropsWithoutRef<T>
> = (props, ref) => {

const refProps = refAble ? { ref } : {};
const renderTimesRef = React.useRef(0);
const prevProps = React.useRef(props);

// If parent has the context, we do not wrap it
const mark = useImmutableMark();
if (mark !== null) {
return <Component {...(props as any)} {...refProps} />;
return <Component {...props} {...refProps} />;
}

if (
// Always trigger re-render if `shouldTriggerRender` is not provided
!shouldTriggerRender ||
shouldTriggerRender(prevProps.current as any, props as any)
shouldTriggerRender(prevProps.current, props)
) {
renderTimesRef.current += 1;
}
Expand All @@ -60,7 +58,7 @@ export default function createImmutable() {

return (
<ImmutableContext.Provider value={renderTimesRef.current}>
<Component {...(props as any)} {...refProps} />
<Component {...props} {...refProps} />
</ImmutableContext.Provider>
);
};
Expand All @@ -70,8 +68,8 @@ export default function createImmutable() {
}

return refAble
? (React.forwardRef(ImmutableComponent) as React.ComponentType<React.ComponentProps<T>>)
: (ImmutableComponent as unknown as React.ComponentType<React.ComponentProps<T>>);
? (React.forwardRef(ImmutableComponent) as unknown as T)
: (ImmutableComponent as T);
}

/**
Expand All @@ -81,13 +79,13 @@ export default function createImmutable() {
function responseImmutable<T extends React.ComponentType<any>>(
Component: T,
propsAreEqual?: CompareProps<T>,
): React.ComponentType<React.ComponentProps<T>> {
): T {
const refAble = supportRef(Component);

const ImmutableComponent = (props: ImmutableProps<T>, ref: React.Ref<any>) => {
const ImmutableComponent: React.ForwardRefRenderFunction<any, any> = (props, ref) => {
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

与 makeImmutable 中的情况类似，这里使用 React.ForwardRefRenderFunction<any, any> 也导致了 props 和 ref 的类型安全性的丧失。

建议使用更精确的类型来定义 ImmutableComponent。

同样，请注意这个改动可能会影响 propsAreEqual 的类型检查。propsAreEqual 的类型是 CompareProps<T>，它期望的参数是 React.ComponentProps<T>。而 React.memo 会传入 ImmutableComponent 的 props，其类型是 React.ComponentPropsWithoutRef<T>，这会导致类型不匹配。这个问题也源于 CompareProps<T> 的定义。

Suggested change
const ImmutableComponent: React.ForwardRefRenderFunction<any, any> = (props, ref) => {
const ImmutableComponent: React.ForwardRefRenderFunction<
React.ElementRef<T>,
React.ComponentPropsWithoutRef<T>
> = (props, ref) => {

const refProps = refAble ? { ref } : {};
useImmutableMark();
return <Component {...(props as any)} {...refProps} />;
return <Component {...props} {...refProps} />;
};

if (process.env.NODE_ENV !== 'production') {
Expand All @@ -96,13 +94,10 @@ export default function createImmutable() {
})`;
}

return refAble
? (React.memo(React.forwardRef(ImmutableComponent), propsAreEqual) as React.ComponentType<
React.ComponentProps<T>
>)
: (React.memo(ImmutableComponent, propsAreEqual) as unknown as React.ComponentType<
React.ComponentProps<T>
>);
return React.memo(
refAble ? React.forwardRef(ImmutableComponent) : ImmutableComponent,
propsAreEqual,
) as unknown as T;
}

return {
Expand Down