chore: move rc-util to @rc-component/util #50
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough将对依赖与构建脚本进行替换（ Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @yoyo837, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request performs a maintenance chore by updating the project's dependency on Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully migrates the dependency from rc-util to @rc-component/util. The changes are straightforward, updating package.json and the corresponding import paths in the source files. I have one minor suggestion for src/context.tsx to sort the new imports alphabetically, which is a common practice for improving code maintainability. Overall, the changes are correct and well-executed.
yoyo837
changed the title
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #50 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 89 91 +2
Branches 24 26 +2
=========================================
+ Hits 89 91 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Immutable.tsx (1)
15-15: 上下文类型不匹配，导致 TS 错误与潜在空值误用默认值是
null，但createContext<number>(null)的泛型不允许null。应显式声明为number | null，与后续mark !== null逻辑一致。- const ImmutableContext = React.createContext<number>(null); + const ImmutableContext = React.createContext<number | null>(null);
🧹 Nitpick comments (1)
src/Immutable.tsx (1)
4-7: 统一“无 ref 的 props”类型，去掉多处as any并匹配 React.memo 比较函数当前：
ImmutableProps<T>通过Omit<..., 'ref'>实现；CompareProps<T>却基于React.ComponentProps<T>；- 导致
shouldTriggerRender(prevProps.current as any, props as any)以及<Component {...(props as any)} .../>等多处as any。建议统一使用
React.ComponentPropsWithoutRef<T>，同时让CompareProps<T>也比较“无 ref 的 props”，从而消除这些as any与类型不一致。- export type CompareProps<T extends React.ComponentType<any>> = ( - prevProps: Readonly<React.ComponentProps<T>>, - nextProps: Readonly<React.ComponentProps<T>>, - ) => boolean; + export type CompareProps<T extends React.ComponentType<any>> = ( + prevProps: Readonly<React.ComponentPropsWithoutRef<T>>, + nextProps: Readonly<React.ComponentPropsWithoutRef<T>>, + ) => boolean; @@ - type ImmutableProps<T extends React.ComponentType<any>> = Omit<React.ComponentProps<T>, 'ref'>; + type ImmutableProps<T extends React.ComponentType<any>> = React.ComponentPropsWithoutRef<T>; @@ - shouldTriggerRender(prevProps.current as any, props as any) + shouldTriggerRender(prevProps.current, props) @@ - return <Component {...(props as any)} {...refProps} />; + return <Component {...(props as any)} {...refProps} />; + // 若上方统一了类型，且 TS 能正确推断，这里的 (props 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> - >); + ? React.memo(React.forwardRef(ImmutableComponent), propsAreEqual) + : React.memo(ImmutableComponent, propsAreEqual);说明：如果担心外部已传入基于带 ref 的比较函数，可在文档中明确
CompareProps的比较目标是“去 ref 的 props”。Based on learningsAlso applies to: 9-10, 54-55, 87-91, 100-105
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(1 hunks)src/Immutable.tsx(6 hunks)src/context.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/context.tsx
- package.json
🔇 Additional comments (1)
src/Immutable.tsx (1)
1-1: 依赖迁移至 @rc-component/util 路径变更正确
supportRef自@rc-component/util/lib/ref引入符合迁移目标与分路径导入最佳实践。
| function makeImmutable<T extends React.ComponentType<any>>( | ||
| Component: T, | ||
| shouldTriggerRender?: CompareProps<T>, | ||
| ): T { | ||
| ): React.ComponentType<React.ComponentProps<T>> { | ||
| const refAble = supportRef(Component); |
There was a problem hiding this comment.
🛠️ 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
| function responseImmutable<T extends React.ComponentType<any>>( | ||
| Component: T, | ||
| propsAreEqual?: CompareProps<T>, | ||
| ): T { | ||
| ): React.ComponentType<React.ComponentProps<T>> { | ||
| const refAble = supportRef(Component); |
There was a problem hiding this comment.
🛠️ 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.
Summary by CodeRabbit