Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 0 additions & 11 deletions now.json

This file was deleted.

9 changes: 4 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,17 @@
"coverage": "father test --coverage",
"lint": "eslint src/ docs/ --ext .tsx,.ts,.jsx,.js",
"now-build": "npm run build",
"prepublishOnly": "npm run compile && np --yolo --no-publish",
"prepublishOnly": "npm run compile && rc-np",
"start": "dumi dev",
"test": "rc-test",
"tsc": "tsc --noEmit"
},
"dependencies": {
"@babel/runtime": "^7.10.1",
"rc-util": "^5.27.0"
"@rc-component/util": "^1.3.0"
},
"devDependencies": {
"@rc-component/father-plugin": "^1.0.0",
"@rc-component/father-plugin": "^2.0.2",
"@rc-component/np": "^1.0.4",
"@types/jest": "^29.5.0",
"@types/react": "^18.0.0",
"@types/react-dom": "^18.0.0",
Expand All @@ -51,7 +51,6 @@
"eslint-plugin-jest": "^28.2.0",
"eslint-plugin-unicorn": "^52.0.0",
"father": "^4.0.0",
"np": "^10.0.4",
"rc-test": "^7.0.14",
"react": "^18.0.0",
"react-dom": "^18.0.0",
Expand Down
35 changes: 21 additions & 14 deletions src/Immutable.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { supportRef } from 'rc-util/lib/ref';
import { supportRef } from '@rc-component/util/lib/ref';
import * as React from 'react';

export type CompareProps<T extends React.ComponentType<any>> = (
prevProps: Readonly<React.ComponentProps<T>>,
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 @@ -32,24 +34,24 @@ export default function createImmutable() {
function makeImmutable<T extends React.ComponentType<any>>(
Component: T,
shouldTriggerRender?: CompareProps<T>,
): T {
): React.ComponentType<React.ComponentProps<T>> {
const refAble = supportRef(Component);
Comment on lines 34 to 38
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

返回类型从原始组件坍缩为 ComponentType 导致 ref 能力丢失,且禁止直接传入 forwardRef 组件

  • 约束 T extends React.ComponentType<any> 会拒绝 React.ForwardRefExoticComponent 与部分 ExoticComponent,这在库场景里是破坏性的。
  • 将返回值强行断言为 React.ComponentType<...> 会丢失 ref 的类型信息(消费端 <Comp ref=... /> 可能报错/无提示)。

建议:为 makeImmutable 提供函数重载以允许 forwardRef 入参,并在 refAble 分支返回真实的 ForwardRefExoticComponent,避免丢失 ref 类型。

-  function makeImmutable<T extends React.ComponentType<any>>(
-    Component: T,
-    shouldTriggerRender?: CompareProps<T>,
-  ): React.ComponentType<React.ComponentProps<T>> {
+  // Overloads: preserve ref typing for forwardRef, and allow normal ComponentType
+  function makeImmutable<T extends React.ForwardRefExoticComponent<any>>(
+    Component: T,
+    shouldTriggerRender?: CompareProps<T>,
+  ): T;
+  function makeImmutable<T extends React.ComponentType<any>>(
+    Component: T,
+    shouldTriggerRender?: CompareProps<T>,
+  ): React.ComponentType<React.ComponentProps<T>>;
+  function makeImmutable<
+    T extends React.ComponentType<any> | React.ForwardRefExoticComponent<any>
+  >(Component: T, shouldTriggerRender?: CompareProps<any>) {
@@
-    return refAble
-      ? (React.forwardRef(ImmutableComponent) as React.ComponentType<React.ComponentProps<T>>)
-      : (ImmutableComponent as unknown as React.ComponentType<React.ComponentProps<T>>);
+    return refAble ? React.forwardRef(ImmutableComponent) : ImmutableComponent;
   }

Also applies to: 72-75


const ImmutableComponent = function (props: any, ref: any) {
const ImmutableComponent = (props: ImmutableProps<T>, ref: React.Ref<any>) => {
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} {...refProps} />;
return <Component {...(props as any)} {...refProps} />;
}

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

return (
<ImmutableContext.Provider value={renderTimesRef.current}>
<Component {...props} {...refProps} />
<Component {...(props as any)} {...refProps} />
</ImmutableContext.Provider>
);
};
Expand All @@ -67,7 +69,9 @@ export default function createImmutable() {
ImmutableComponent.displayName = `ImmutableRoot(${Component.displayName || Component.name})`;
}

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

/**
Expand All @@ -77,14 +81,13 @@ export default function createImmutable() {
function responseImmutable<T extends React.ComponentType<any>>(
Component: T,
propsAreEqual?: CompareProps<T>,
): T {
): React.ComponentType<React.ComponentProps<T>> {
const refAble = supportRef(Component);
Comment on lines 81 to 85
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

responseImmutable 同样存在 ref 与类型坍缩问题;memo 比较函数的入参与内部 props 类型不一致

  • 目前返回被断言为 React.ComponentType<...>,会丢失 memo/forwardRef 的精确信息。
  • propsAreEqual 的签名使用的是 React.ComponentProps<T>,但这里 memo 的是 ImmutableComponent(其 props 已去掉了 ref),存在类型不一致。

建议:添加重载以保留 memo(forwardRef(...)) 的精准类型,并让比较函数对“无 ref 的 props”进行比较。

-  function responseImmutable<T extends React.ComponentType<any>>(
-    Component: T,
-    propsAreEqual?: CompareProps<T>,
-  ): React.ComponentType<React.ComponentProps<T>> {
+  // Overloads: preserve accurate exotic component types
+  function responseImmutable<T extends React.ForwardRefExoticComponent<any>>(
+    Component: T,
+    propsAreEqual?: CompareProps<T>,
+  ): React.MemoExoticComponent<T>;
+  function responseImmutable<T extends React.ComponentType<any>>(
+    Component: T,
+    propsAreEqual?: CompareProps<T>,
+  ): React.MemoExoticComponent<React.ComponentType<React.ComponentProps<T>>>;
+  function responseImmutable<
+    T extends React.ComponentType<any> | React.ForwardRefExoticComponent<any>
+  >(Component: T, propsAreEqual?: CompareProps<any>) {
@@
-    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 refAble
+      ? React.memo(React.forwardRef(ImmutableComponent), propsAreEqual)
+      : React.memo(ImmutableComponent, propsAreEqual);
   }

Also applies to: 100-105

🤖 Prompt for AI Agents
In src/Immutable.tsx around lines 81-85 (also apply same changes to 100-105):
responseImmutable currently collapses precise memo/forwardRef types and
mis-signatures the comparator by using React.ComponentProps<T> (which still
includes ref) while the internal ImmutableComponent strips ref; fix by adding
overloads for responseImmutable to preserve exact return types when T is a
ForwardRefExoticComponent/Ref/MutableRef—i.e. provide a signature that returns
the same ForwardRefExoticComponent/Named memo-wrapped type when a ref-supporting
component is passed—change propsAreEqual to accept props with ref removed (use
OmitRefProps or JSX.LibraryManagedAttributes/RemoveRef wrapper type) and use
correct generics when calling React.memo and React.forwardRef so TypeScript can
infer the precise combined type rather than collapsing to ComponentType; update
internal typing around supportRef/ref handling to map T -> PropsWithoutRef and
ensure the exported return type matches the specific memo/forwardRef type rather
than generic ComponentType.


const ImmutableComponent = function (props: any, ref: any) {
const ImmutableComponent = (props: ImmutableProps<T>, ref: React.Ref<any>) => {
const refProps = refAble ? { ref } : {};
useImmutableMark();

return <Component {...props} {...refProps} />;
return <Component {...(props as any)} {...refProps} />;
};

if (process.env.NODE_ENV !== 'production') {
Expand All @@ -94,8 +97,12 @@ export default function createImmutable() {
}

return refAble
? React.memo(React.forwardRef(ImmutableComponent), propsAreEqual)
: (React.memo(ImmutableComponent, propsAreEqual) as any);
? (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 {
Expand Down
6 changes: 3 additions & 3 deletions src/context.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import useEvent from 'rc-util/lib/hooks/useEvent';
import useLayoutEffect from 'rc-util/lib/hooks/useLayoutEffect';
import isEqual from 'rc-util/lib/isEqual';
import useEvent from '@rc-component/util/lib/hooks/useEvent';
import useLayoutEffect from '@rc-component/util/lib/hooks/useLayoutEffect';
import isEqual from '@rc-component/util/lib/isEqual';
import * as React from 'react';
import { unstable_batchedUpdates } from 'react-dom';

Expand Down
12 changes: 3 additions & 9 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,9 @@
"skipLibCheck": true,
"esModuleInterop": true,
"paths": {
"@/*": [
"src/*"
],
"@@/*": [
".dumi/tmp/*"
],
"@rc-component/context": [
"src/index.ts"
]
"@/*": ["src/*"],
"@@/*": [".dumi/tmp/*"],
"@rc-component/context": ["src/index.ts"]
}
},
"include": [".dumi/**/*", ".dumirc.ts", "**/*.ts", "**/*.tsx"]
Expand Down