Skip to content
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.

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

feat(form): add useWatch #2932

Open
wants to merge 5 commits into
base: next
Choose a base branch
from

Conversation

oasis-cloud
Copy link
Collaborator

@oasis-cloud oasis-cloud commented Jan 9, 2025

🤔 这个变动的性质是?

  • 新特性提交

🔗 相关 Issue

#2216

Summary by CodeRabbit

  • 新功能

    • 在表单组件中新增 useWatch 方法,允许监听和追踪特定表单输入的变化
    • 添加了一个新的登录方法演示,支持手机号和邮箱两种登录方式
  • 文档

    • 更新了表单组件的文档,详细说明了 useWatch 方法的使用和功能
    • 在文档中新增了关于 useWatch 的章节,提供了示例代码和方法描述
  • 改进

    • 优化了表单状态管理,增强了组件的响应性和交互性

Copy link

coderabbitai bot commented Jan 9, 2025

"""

演练

此拉取请求引入了一个新的表单演示组件 Demo8,展示了 Form.useWatch() 钩子的使用。更改涉及多个文件,包括在不同平台(H5 和 Taro)上添加演示组件、更新文档和扩展表单组件的功能。新组件允许用户选择登录方式(手机或邮箱),并动态显示所选账户信息。

变更

文件路径 变更摘要
src/packages/form/demo.taro.tsx
src/packages/form/demo.tsx
导入 Demo8 组件并添加到 FormDemo 中,新增翻译键 useWatch
src/packages/form/demos/h5/demo8.tsx
src/packages/form/demos/taro/demo8.tsx
新增 Demo8 组件,演示 useWatch 钩子功能
src/packages/form/doc*.md 添加 useWatch 部分,包括方法描述和示例
src/packages/form/index*.ts 更新 CompoundedComponent 类型,添加 useWatch 方法
src/packages/form/useform*.ts 实现 useWatch 钩子和相关方法
src/packages/form/__tests__/form.spec.tsx 添加 useWatch 功能的测试用例

序列图

sequenceDiagram
    participant User
    participant Form
    participant UseWatch
    
    User->>Form: 选择登录方式
    Form->>UseWatch: 监听字段变化
    UseWatch-->>Form: 更新表单状态
    Form->>User: 动态显示账户信息
Loading

可能相关的 PR

建议标签

size/XL

建议审阅者

  • xiaoyatong
  • Alex-huxiyang
  • irisSong

诗歌

🐰 表单跳舞,监听欢歌
手机邮箱,数据流淌
useWatch 魔法,状态轻盈
用户体验,如兔子般灵动
代码之舞,悄然绽放 🌟
"""

Finishing Touches

  • 📝 Generate Docstrings (Beta)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the 2.x label Jan 9, 2025
Copy link

codecov bot commented Jan 9, 2025

Codecov Report

Attention: Patch coverage is 93.47826% with 3 lines in your changes missing coverage. Please review.

Project coverage is 84.15%. Comparing base (5ce15cd) to head (1b615f7).
Report is 5 commits behind head on next.

Files with missing lines Patch % Lines
src/packages/form/useform.ts 93.18% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             next    #2932      +/-   ##
==========================================
+ Coverage   84.02%   84.15%   +0.12%     
==========================================
  Files         220      267      +47     
  Lines       17917    18058     +141     
  Branches     2628     2641      +13     
==========================================
+ Hits        15055    15196     +141     
  Misses       2857     2857              
  Partials        5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (3)
src/packages/form/useform.ts (1)

270-281: 优化 notifyWatch 实现

当前实现在某些情况下会重复获取表单值。

建议修改如下:

 private notifyWatch = (namePath: NamePath[] = []) => {
   if (this.watchList.length) {
-    let allValues = this.getFieldsValue(namePath)
-
-    if (!namePath || namePath.length === 0) {
-      allValues = this.getFieldsValue(true)
-    }
+    const allValues = this.getFieldsValue(true)
     this.watchList.forEach((callback) => {
       callback(allValues, namePath)
     })
   }
 }
src/packages/form/demos/h5/demo8.tsx (2)

16-37: 移除不必要的 Fragment

footer 属性中使用了不必要的 Fragment 包装。

建议修改如下:

 footer={
-  <>
   <div
     style={{
       width: '100%',
     }}
   >
     <div
       style={{
         fontSize: '12px',
         textAlign: 'center',
         marginBottom: '20px',
       }}
     >
       你将使用 {loginMethod === 'mobile' ? '手机号' : '邮箱'} {account}{' '}
       登录
     </div>
     <Button block type="primary" size="large" nativeType="submit">
       提交
     </Button>
   </div>
-  </>
 }
🧰 Tools
🪛 Biome (1.9.4)

[error] 16-39: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)


48-59: 优化条件渲染逻辑

当前实现使用了重复的 Form.Item 结构,可以通过提取共同部分来优化。

建议修改如下:

-      <>
-        {loginMethod === 'mobile' && (
-          <Form.Item name="account" label="手机号">
-            <Input placeholder="请输入手机号" />
-          </Form.Item>
-        )}
-        {loginMethod === 'email' && (
-          <Form.Item name="account" label="邮箱">
-            <Input placeholder="请输入邮箱" />
-          </Form.Item>
-        )}
-      </>
+      <Form.Item
+        name="account"
+        label={loginMethod === 'mobile' ? '手机号' : '邮箱'}
+      >
+        <Input
+          placeholder={`请输入${loginMethod === 'mobile' ? '手机号' : '邮箱'}`}
+        />
+      </Form.Item>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5ce15cd and 71a719d.

📒 Files selected for processing (12)
  • src/packages/form/demo.taro.tsx (4 hunks)
  • src/packages/form/demo.tsx (4 hunks)
  • src/packages/form/demos/h5/demo8.tsx (1 hunks)
  • src/packages/form/demos/taro/demo8.tsx (1 hunks)
  • src/packages/form/doc.en-US.md (3 hunks)
  • src/packages/form/doc.md (3 hunks)
  • src/packages/form/doc.taro.md (3 hunks)
  • src/packages/form/doc.zh-TW.md (3 hunks)
  • src/packages/form/index.taro.ts (2 hunks)
  • src/packages/form/index.ts (2 hunks)
  • src/packages/form/useform.taro.ts (7 hunks)
  • src/packages/form/useform.ts (7 hunks)
🧰 Additional context used
🪛 LanguageTool
src/packages/form/doc.en-US.md

[uncategorized] ~143-~143: Loose punctuation mark.
Context: ...lidation | Promise | Form.useWath(), this method will watch specified inputs...

(UNLIKELY_OPENING_PUNCTUATION)

🪛 Biome (1.9.4)
src/packages/form/demos/taro/demo8.tsx

[error] 16-39: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

src/packages/form/demos/h5/demo8.tsx

[error] 16-39: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

🪛 GitHub Check: build
src/packages/form/demos/taro/demo8.tsx

[failure] 7-7:
Expected 0 type arguments, but got 1.

src/packages/form/demos/h5/demo8.tsx

[failure] 7-7:
Expected 0 type arguments, but got 1.

src/packages/form/useform.ts

[failure] 262-262:
Parameter 'callback' implicitly has an 'any' type.

src/packages/form/useform.taro.ts

[failure] 262-262:
Parameter 'callback' implicitly has an 'any' type.

🪛 GitHub Actions: CI
src/packages/form/demos/h5/demo8.tsx

[error] 7-7: Expected 0 type arguments, but got 1

🔇 Additional comments (10)
src/packages/form/index.ts (1)

5-5: 导出 useWatch 钩子函数的实现看起来很合理!

代码结构清晰,与现有的 useForm 实现保持一致的模式。

Also applies to: 20-20, 27-27

src/packages/form/index.taro.ts (1)

5-5: Taro 版本的 useWatch 实现与标准版保持一致,很好!

代码实现保持了与非 Taro 版本相同的结构和模式,确保了跨平台的一致性。

Also applies to: 20-20, 27-27

src/packages/form/demo.tsx (1)

10-10: 新增的 useWatch 示例集成正确!

示例代码的集成遵循了现有的模式,包括了中英文的翻译支持。

让我们验证一下 Demo8 组件是否存在:

Also applies to: 22-22, 32-32, 53-54

✅ Verification successful

Demo8 组件已正确集成

验证确认 Demo8 组件文件存在于正确的位置。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 验证 Demo8 组件文件是否存在
fd "demo8.tsx" src/packages/form/demos/h5/

Length of output: 79

src/packages/form/demo.taro.tsx (2)

Line range hint 1-1: 建议验证 useWatch 的具体实现

让我们检查一下 useWatch 的实现文件:

✅ Verification successful

useWatch 实现已验证完成,代码实现正确

标准版本和 Taro 版本的 useWatch 实现完全一致,均采用了合理的 React Hooks 模式,包含了必要的状态管理和表单实例监听逻辑。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查 useWatch 的实现
echo "检查标准版本的 useWatch 实现:"
rg -A 10 "useWatch" src/packages/form/useform.ts

echo "检查 Taro 版本的 useWatch 实现:"
rg -A 10 "useWatch" src/packages/form/useform.taro.ts

Length of output: 802


12-12: Taro 版本的 useWatch 示例同样集成得当!

示例代码与标准版保持一致,确保了良好的跨平台体验。

让我们验证一下 Taro 版本的 Demo8 组件是否存在:

Also applies to: 24-24, 34-34, 56-57

✅ Verification successful

已确认 Taro 版本的 Demo8 组件集成完整

文件 src/packages/form/demos/taro/demo8.tsx 已正确存在于预期位置。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 验证 Taro 版本的 Demo8 组件文件是否存在
fd "demo8.tsx" src/packages/form/demos/taro/

Length of output: 83

src/packages/form/useform.taro.ts (1)

Line range hint 14-305: 实现与 useform.ts 相同的问题

此文件的实现与 useform.ts 完全相同,因此存在相同的问题:

  1. registerWatch 缺少类型声明
  2. useWatch 需要清理订阅
  3. notifyWatch 实现可以优化

请参考 useform.ts 的修改建议。

🧰 Tools
🪛 GitHub Check: build

[failure] 262-262:
Parameter 'callback' implicitly has an 'any' type.

src/packages/form/demos/taro/demo8.tsx (1)

1-64: 实现与 H5 版本相同的问题

此文件的实现与 H5 版本完全相同,因此存在相同的问题:

  1. footer 中存在不必要的 Fragment
  2. 条件渲染逻辑可以优化

请参考 H5 版本的修改建议。

🧰 Tools
🪛 Biome (1.9.4)

[error] 16-39: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

🪛 GitHub Check: build

[failure] 7-7:
Expected 0 type arguments, but got 1.

src/packages/form/doc.zh-TW.md (1)

61-68: 文档结构清晰,内容完整!

新增的 useWatch 部分组织得当,与其他语言版本保持一致。

Also applies to: 142-143

src/packages/form/doc.md (1)

61-68: 文档保持了良好的一致性!

新增的 useWatch 部分与繁体中文版本保持一致,结构清晰。

Also applies to: 142-143

src/packages/form/doc.taro.md (1)

61-68: Taro 文档适配正确!

新增的 useWatch 部分正确使用了 Taro 版本的示例路径(taro/demo8.tsx),保持了与其他版本的内容一致性。

Also applies to: 142-143

src/packages/form/useform.ts Outdated Show resolved Hide resolved
src/packages/form/useform.ts Outdated Show resolved Hide resolved
src/packages/form/doc.en-US.md Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/packages/form/useform.ts (1)

297-305: ⚠️ Potential issue

useWatch 需要添加清理逻辑

当前实现在组件卸载时没有清理订阅,可能导致内存泄漏。

建议修改如下:

export const useWatch = (path: NamePath, form: FormInstance) => {
  const formInstance = form.getInternal(SECRET)
  const [value, setValue] = useState<any>()
-  formInstance.registerWatch((data: any, namePath: NamePath) => {
+  useEffect(() => {
+    const unsubscribe = formInstance.registerWatch((data: any, namePath: NamePath) => {
      const value = data[path]
      setValue(value)
-  })
+    })
+    return () => {
+      unsubscribe()
+    }
+  }, [formInstance, path])
  return value
}
🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 298-305: src/packages/form/useform.ts#L298-L305
Added lines #L298 - L305 were not covered by tests

🧹 Nitpick comments (6)
src/packages/form/useform.taro.ts (3)

94-94: 建议优化初始化时的通知机制

当前实现在设置初始值时通知所有监听器,但没有传递具体的字段路径。建议传递受影响的字段列表,以便监听器能够更精确地处理变更。

- this.notifyWatch()
+ this.notifyWatch(Object.keys(initialValues))

270-281: 建议优化大型表单场景下的性能

当前实现在每次通知时都会重新获取所有表单值,这在大型表单中可能造成性能问题。建议:

  1. 实现值缓存机制
  2. 添加防抖处理
 private notifyWatch = (namePath: NamePath[] = []) => {
   if (this.watchList.length) {
-    let allValues = this.getFieldsValue(namePath)
+    // 使用防抖包装获取值的操作
+    const debouncedGetValues = debounce(() => {
+      let allValues = this.getFieldsValue(namePath)
 
-    if (!namePath || namePath.length === 0) {
-      allValues = this.getFieldsValue(true)
-    }
-    this.watchList.forEach((callback) => {
-      callback(allValues, namePath)
-    })
+      if (!namePath || namePath.length === 0) {
+        allValues = this.getFieldsValue(true)
+      }
+      this.watchList.forEach((callback) => {
+        callback(allValues, namePath)
+      })
+    }, 16) // 约一帧的时间
+    
+    debouncedGetValues()
   }
 }

1-1: 补充缺失的 useEffect 导入

需要从 React 中导入 useEffect。

- import { useRef, useState } from 'react'
+ import { useRef, useState, useEffect } from 'react'
src/packages/form/demos/h5/demo8.tsx (3)

4-4: 建议改进类型定义的严谨性

建议将类型定义修改为更严格的形式,使用 Required 类型确保字段必填:

-type FieldType = { account?: string; loginMethod?: 'mobile' | 'email' }
+type FieldType = Required<{
+  account: string
+  loginMethod: 'mobile' | 'email'
+}>

15-37: 建议优化代码结构和样式管理

  1. 移除不必要的 Fragment
  2. 建议将内联样式提取到单独的样式文件中
footer={
-  <>
    <div
      style={{
        width: '100%',
      }}
    >
-    <div
-      style={{
-        fontSize: '12px',
-        textAlign: 'center',
-        marginBottom: '20px',
-      }}
-    >
+    <div className="login-message">
      你将使用 {loginMethod === 'mobile' ? '手机号' : '邮箱'} {account}{' '}
      登录
    </div>
    <Button block type="primary" size="large" nativeType="submit">
      提交
    </Button>
    </div>
-  </>
}

建议创建对应的样式文件:

.login-message {
  font-size: 12px;
  text-align: center;
  margin-bottom: 20px;
}

48-59: 建议优化表单字段的条件渲染逻辑

当前实现中存在不必要的 Fragment 和重复的 Form.Item 逻辑,建议优化如下:

-<>
-  {loginMethod === 'mobile' && (
-    <Form.Item name="account" label="手机号">
-      <Input placeholder="请输入手机号" />
-    </Form.Item>
-  )}
-  {loginMethod === 'email' && (
-    <Form.Item name="account" label="邮箱">
-      <Input placeholder="请输入邮箱" />
-    </Form.Item>
-  )}
-</>
+<Form.Item
+  name="account"
+  label={loginMethod === 'mobile' ? '手机号' : '邮箱'}
+>
+  <Input
+    placeholder={`请输入${loginMethod === 'mobile' ? '手机号' : '邮箱'}`}
+  />
+</Form.Item>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 71a719d and 626e22e.

📒 Files selected for processing (4)
  • src/packages/form/demos/h5/demo8.tsx (1 hunks)
  • src/packages/form/demos/taro/demo8.tsx (1 hunks)
  • src/packages/form/useform.taro.ts (7 hunks)
  • src/packages/form/useform.ts (7 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/form/useform.ts

[warning] 130-130: src/packages/form/useform.ts#L130
Added line #L130 was not covered by tests


[warning] 263-263: src/packages/form/useform.ts#L263
Added line #L263 was not covered by tests


[warning] 265-268: src/packages/form/useform.ts#L265-L268
Added lines #L265 - L268 were not covered by tests


[warning] 272-272: src/packages/form/useform.ts#L272
Added line #L272 was not covered by tests


[warning] 274-280: src/packages/form/useform.ts#L274-L280
Added lines #L274 - L280 were not covered by tests


[warning] 298-305: src/packages/form/useform.ts#L298-L305
Added lines #L298 - L305 were not covered by tests

🪛 Biome (1.9.4)
src/packages/form/demos/h5/demo8.tsx

[error] 16-39: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

src/packages/form/demos/taro/demo8.tsx

[error] 16-39: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build
🔇 Additional comments (6)
src/packages/form/useform.taro.ts (2)

14-14: 类型定义清晰且合理!

回调函数的类型定义准确地描述了监听器的参数类型,包括表单数据和变更字段路径。


260-268: 监听器注册机制实现合理!

注册机制包含了适当的清理函数,可以有效防止内存泄漏。

src/packages/form/useform.ts (2)

14-14: 类型定义清晰完整!

WatchCallback 类型定义明确指定了回调函数的参数类型,有助于类型安全。


260-281: 需要添加测试用例

监听相关的代码缺少测试覆盖,建议添加以下场景的测试:

  • watchList 的注册和注销
  • notifyWatch 在不同参数下的行为
  • 值变更时的回调触发

建议添加以下测试场景:

describe('watch functionality', () => {
  test('should register and unregister watch callbacks', () => {
    // 测试注册和注销逻辑
  });
  
  test('should notify watchers with correct values', () => {
    // 测试值变更通知
  });
  
  test('should handle empty namePath correctly', () => {
    // 测试空路径处理
  });
});
✅ Verification successful

确认需要添加 watch 相关测试用例

经过验证,当前测试文件 src/packages/form/__tests__/form.spec.tsx 中确实缺少对 watch 功能的测试覆盖。建议按照现有测试风格,在该文件中添加以下测试场景:

  • 测试 watch 回调的注册和注销机制
  • 测试表单值变更时 watch 回调的触发情况
  • 测试不同 namePath 参数下的 watch 行为

示例测试结构:

test('watch callback registration and notification', async () => {
  const watchCallback = vi.fn()
  // 测试实现...
})
🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Find test files related to useform
fd -e test.ts -e spec.ts useform

# Search for existing watch-related tests
rg -l "describe\(.*watch" --type ts

# Look for test patterns in form package
rg "test\(.*\)" "src/packages/form" --type ts -A 2

Length of output: 4327

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 263-263: src/packages/form/useform.ts#L263
Added line #L263 was not covered by tests


[warning] 265-268: src/packages/form/useform.ts#L265-L268
Added lines #L265 - L268 were not covered by tests


[warning] 272-272: src/packages/form/useform.ts#L272
Added line #L272 was not covered by tests


[warning] 274-280: src/packages/form/useform.ts#L274-L280
Added lines #L274 - L280 were not covered by tests

src/packages/form/demos/h5/demo8.tsx (1)

7-9: 代码实现规范,值得肯定!

Form.useForm 和 Form.useWatch 的使用方式正确,遵循了 React Hooks 的最佳实践。

src/packages/form/demos/taro/demo8.tsx (1)

1-64: 建议与 H5 版本保持一致的优化

该文件与 H5 版本基本相同,建议应用相同的优化建议:

  1. 优化类型定义
  2. 移除不必要的 Fragment
  3. 提取内联样式
  4. 简化条件渲染逻辑
  5. 添加表单验证规则

另外,建议考虑将共享逻辑提取到一个公共的 hooks 文件中,以减少代码重复:

// src/packages/form/hooks/useLoginForm.ts
import { Form } from '@nutui/nutui-react'

export const useLoginForm = () => {
  const [form] = Form.useForm()
  const account = Form.useWatch('account', form)
  const loginMethod = Form.useWatch('loginMethod', form)

  const getValidationRules = () => [
    { required: true, message: '请输入账号' },
    {
      validator: (_, value) => {
        if (!value) return Promise.resolve()
        if (loginMethod === 'mobile') {
          return /^1[3-9]\d{9}$/.test(value)
            ? Promise.resolve()
            : Promise.reject('请输入正确的手机号')
        }
        return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(value)
          ? Promise.resolve()
          : Promise.reject('请输入正确的邮箱地址')
      },
    },
  ]

  return {
    form,
    account,
    loginMethod,
    getValidationRules,
  }
}
🧰 Tools
🪛 Biome (1.9.4)

[error] 16-39: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

src/packages/form/useform.taro.ts Outdated Show resolved Hide resolved
src/packages/form/demos/h5/demo8.tsx Outdated Show resolved Hide resolved
@coderabbitai coderabbitai bot added the size/XL label Jan 15, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (3)
src/packages/form/__tests__/form.spec.tsx (1)

362-373: 移除不必要的 Fragment

当前代码使用了不必要的 Fragment 包裹条件渲染的表单项。由于这些表单项是互斥的,可以直接使用条件运算符。

建议修改如下:

-<>
-  {loginMethod === 'mobile' && (
-    <Form.Item name="account" label="手机号">
-      <Input placeholder="请输入手机号" />
-    </Form.Item>
-  )}
-  {loginMethod === 'email' && (
-    <Form.Item name="account" label="邮箱">
-      <Input placeholder="请输入邮箱" />
-    </Form.Item>
-  )}
-</>
+{loginMethod === 'mobile' ? (
+  <Form.Item name="account" label="手机号">
+    <Input placeholder="请输入手机号" />
+  </Form.Item>
+) : (
+  <Form.Item name="account" label="邮箱">
+    <Input placeholder="请输入邮箱" />
+  </Form.Item>
+)}
src/packages/form/doc.en-US.md (1)

143-144: 改进文档格式和可读性

建议对文档格式进行以下改进:

  1. 添加代码示例说明 useWatch 的用法
  2. 添加参数说明表格
  3. 添加返回值说明

建议修改为:

### useWatch

`Form.useWatch(name: NamePath, form: FormInstance)` 用于监听表单字段值的变化。

#### 参数

| 参数 | 说明 | 类型 | 默认值 |
| --- | --- | --- | --- |
| name | 需要监听的字段名 | `NamePath` | - |
| form | 表单实例 | `FormInstance` | - |

#### 返回值

返回被监听字段的当前值。

#### 示例

```tsx
const Demo = () => {
  const [form] = Form.useForm()
  const value = Form.useWatch('field', form)
  return <div>{value}</div>
}

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 LanguageTool</summary>

[uncategorized] ~143-~143: Loose punctuation mark.
Context: ...idation | `Promise` |  `Form.useWatch()`, this method will watch specified inputs...

(UNLIKELY_OPENING_PUNCTUATION)

</details>

</details>

</blockquote></details>
<details>
<summary>src/packages/form/useform.taro.ts (1)</summary><blockquote>

`275-287`: **优化 notifyWatch 性能**

当前实现在每次通知时都会创建新的值对象,可以通过以下优化减少不必要的计算:
1. 缓存 `getFieldsValue` 的结果
2. 只在确实需要时才获取所有值


建议修改如下:

```typescript
private notifyWatch = (namePath: NamePath[] = []) => {
  if (!this.watchList.length) return;
  
  const values = namePath.length > 0 
    ? this.getFieldsValue(namePath)
    : this.getFieldsValue(true);
    
  this.watchList.forEach((callback) => {
    callback(values, namePath);
  });
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 626e22e and d5fe0db.

📒 Files selected for processing (4)
  • src/packages/form/__tests__/form.spec.tsx (2 hunks)
  • src/packages/form/doc.en-US.md (3 hunks)
  • src/packages/form/useform.taro.ts (8 hunks)
  • src/packages/form/useform.ts (8 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/form/__tests__/form.spec.tsx

[error] 330-353: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

🪛 GitHub Check: codecov/patch
src/packages/form/useform.ts

[warning] 135-135: src/packages/form/useform.ts#L135
Added line #L135 was not covered by tests


[warning] 281-282: src/packages/form/useform.ts#L281-L282
Added lines #L281 - L282 were not covered by tests

🪛 LanguageTool
src/packages/form/doc.en-US.md

[uncategorized] ~143-~143: Loose punctuation mark.
Context: ...idation | Promise | Form.useWatch(), this method will watch specified inputs...

(UNLIKELY_OPENING_PUNCTUATION)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build
🔇 Additional comments (2)
src/packages/form/useform.ts (2)

275-287: 优化 notifyWatch 性能

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 281-282: src/packages/form/useform.ts#L281-L282
Added lines #L281 - L282 were not covered by tests


303-320: 完善 useWatch 实现

src/packages/form/__tests__/form.spec.tsx Show resolved Hide resolved
src/packages/form/useform.taro.ts Show resolved Hide resolved
Copy link
Collaborator

@xiaoyatong xiaoyatong left a comment

Choose a reason for hiding this comment

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

切换回来的时候,数据被重置了~

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (7)
src/packages/form/demos/h5/demo8.tsx (2)

6-10: 建议添加错误处理机制

当前的 useWatch 实现没有错误处理机制。建议添加 try-catch 来优雅处理可能出现的异常。

 const Demo8 = () => {
   const [form] = Form.useForm()
-  const account = Form.useWatch('account', form)
-  const loginMethod = Form.useWatch('loginMethod', form)
+  const account = Form.useWatch('account', form) ?? ''
+  const loginMethod = Form.useWatch('loginMethod', form) ?? 'mobile'

15-37: 优化表单页脚结构

  1. 移除不必要的 Fragment 包裹
  2. 简化嵌套的 div 结构,使用更语义化的标签
   footer={
-    <>
-      <div
-        style={{
-          width: '100%',
-        }}
-      >
-        <div
-          style={{
-            fontSize: '12px',
-            textAlign: 'center',
-            marginBottom: '20px',
-          }}
-        >
+    <section style={{ width: '100%' }}>
+      <p
+        style={{
+          fontSize: '12px',
+          textAlign: 'center',
+          marginBottom: '20px',
+        }}
+      >
         你将使用{loginMethod === 'mobile' ? '手机号' : '电子邮箱'}
         {account}登录
-        </div>
-        <Button block type="primary" size="large" nativeType="submit">
-          提交
-        </Button>
-      </div>
-    </>
+      </p>
+      <Button block type="primary" size="large" nativeType="submit">
+        提交
+      </Button>
+    </section>
   }
src/packages/form/demos/taro/demo8.tsx (3)

4-4: 建议改进类型定义的严谨性

建议将可选属性改为必需属性,因为表单初始值已经确保这些字段存在。

-type FieldType = { account?: string; loginMethod?: 'mobile' | 'email' }
+type FieldType = { account: string; loginMethod: 'mobile' | 'email' }

16-37: 优化代码结构和样式管理

代码存在以下可以改进的地方:

  1. 不必要的 Fragment 包裹
  2. 内联样式应该提取到样式文件中

建议按如下方式修改:

-      footer={
-        <>
+      footer={
           <div
             style={{
               width: '100%',
             }}
           >
             {/* ... */}
           </div>
-        </>
       }

另外,建议将内联样式提取到单独的样式文件中:

// styles.ts
export const footerStyles = {
  container: {
    width: '100%',
  },
  message: {
    fontSize: '12px',
    textAlign: 'center',
    marginBottom: '20px',
  },
}
🧰 Tools
🪛 Biome (1.9.4)

[error] 16-39: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)


48-59: 建议优化条件渲染逻辑

当前实现中存在重复的 Form.Item 结构,可以通过提取共同部分来优化代码。

-      <>
-        {loginMethod === 'mobile' && (
-          <Form.Item name="account" label="手机号">
-            <Input placeholder="请输入手机号" />
-          </Form.Item>
-        )}
-        {loginMethod === 'email' && (
-          <Form.Item name="account" label="电子邮箱">
-            <Input placeholder="请输入邮箱" />
-          </Form.Item>
-        )}
-      </>
+      <Form.Item
+        name="account"
+        label={loginMethod === 'mobile' ? '手机号' : '电子邮箱'}
+      >
+        <Input
+          placeholder={`请输入${loginMethod === 'mobile' ? '手机号' : '邮箱'}`}
+        />
+      </Form.Item>
src/packages/form/useform.taro.ts (1)

259-281: 监听器实现完善,建议优化值比较逻辑

监听器的实现整体良好,包括注册、清理和通知机制。建议在通知监听器之前添加值比较,避免不必要的更新。

 private notifyWatch = (namePath: NamePath[] = []) => {
   if (this.watchList.length) {
     let allValues
     if (!namePath || namePath.length === 0) {
       allValues = this.getFieldsValue(true)
     } else {
       allValues = this.getFieldsValue(namePath)
     }
     this.watchList.forEach((callback) => {
+      // 添加值比较,避免不必要的回调
+      const prevValue = this.store[namePath]
+      if (JSON.stringify(prevValue) !== JSON.stringify(allValues)) {
         callback(allValues, namePath)
+      }
     })
   }
 }
src/packages/form/useform.ts (1)

297-314: 建议优化初始值设置逻辑!

当前实现在 useEffect 中获取初始值并设置,这可能导致不必要的重渲染。建议优化如下:

 export const useWatch = (path: NamePath, form: FormInstance) => {
   const formInstance = form.getInternal(SECRET)
-  const [value, setValue] = useState<any>()
+  const [value, setValue] = useState(() => {
+    const initialValue = form.getFieldsValue(true)
+    return initialValue[path]
+  })
   useEffect(() => {
     const unsubscribe = formInstance.registerWatch(
       (data: any, namePath: NamePath) => {
         const value = data[path]
         setValue(value)
       }
     )
-    const initialValue = form.getFieldsValue(true)
-    if (value !== initialValue[path]) {
-      setValue(initialValue[path])
-    }
     return () => unsubscribe()
   }, [form])
   return value
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d5fe0db and 1b615f7.

📒 Files selected for processing (4)
  • src/packages/form/demos/h5/demo8.tsx (1 hunks)
  • src/packages/form/demos/taro/demo8.tsx (1 hunks)
  • src/packages/form/useform.taro.ts (8 hunks)
  • src/packages/form/useform.ts (8 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/form/useform.ts

[warning] 129-129: src/packages/form/useform.ts#L129
Added line #L129 was not covered by tests


[warning] 275-276: src/packages/form/useform.ts#L275-L276
Added lines #L275 - L276 were not covered by tests

🪛 Biome (1.9.4)
src/packages/form/demos/h5/demo8.tsx

[error] 16-39: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

src/packages/form/demos/taro/demo8.tsx

[error] 16-39: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build
🔇 Additional comments (9)
src/packages/form/demos/h5/demo8.tsx (2)

1-5: 代码结构清晰,类型定义合理!

导入语句简洁,类型定义准确。


48-59: 重申:建议添加表单验证规则

当前实现仍然缺少必要的表单验证,建议添加手机号和邮箱的格式验证。

src/packages/form/demos/taro/demo8.tsx (1)

6-10: 代码实现清晰简洁

Form.useWatch 的使用恰当,能够有效监听表单字段的变化。

src/packages/form/useform.taro.ts (4)

14-14: 类型定义清晰且类型安全

WatchCallback 类型定义明确了监听回调函数的参数类型和返回值,有助于代码的类型安全性。


93-93: 状态更新通知机制集成合理

在所有表单状态更新的关键位置(初始化、字段更新、单字段更新)都添加了 notifyWatch 调用,确保了监听器能及时获得状态变化通知。

Also applies to: 121-121, 129-129


240-240: 内部 API 扩展合理

通过 getInternal 方法暴露 registerWatch 功能,保持了良好的封装性同时提供必要的访问能力。


297-314: ⚠️ Potential issue

useWatch 钩子实现需要改进

当前实现存在以下问题:

  1. useEffect 依赖数组缺少 path
  2. 值更新时没有比较,可能导致不必要的重渲染
  3. 初始值的处理可以优化

建议按如下方式修改实现:

 export const useWatch = (path: NamePath, form: FormInstance) => {
   const formInstance = form.getInternal(SECRET)
-  const [value, setValue] = useState<any>()
+  const [value, setValue] = useState(() => {
+    const initialValue = form.getFieldsValue(true)
+    return initialValue[path]
+  })
   useEffect(() => {
     const unsubscribe = formInstance.registerWatch(
       (data: any, namePath: NamePath) => {
         const value = data[path]
-        setValue(value)
+        if (JSON.stringify(value) !== JSON.stringify(data[path])) {
+          setValue(value)
+        }
       }
     )
-    const initialValue = form.getFieldsValue(true)
-    if (value !== initialValue[path]) {
-      setValue(initialValue[path])
-    }
     return () => unsubscribe()
-  }, [form])
+  }, [form, path])
   return value
 }

Likely invalid or redundant comment.

src/packages/form/useform.ts (2)

14-14: 类型定义清晰完整!

WatchCallback 类型定义明确指定了回调函数的参数类型,有助于提供良好的类型安全性。


93-93: 状态更新通知机制集成完善!

在所有状态更新的关键点(setInitialValuessetFieldsValuesetFieldValue)都正确添加了 notifyWatch 调用,确保了表单值变化时的实时通知。

Also applies to: 121-121, 129-129

src/packages/form/useform.ts Show resolved Hide resolved
@oasis-cloud oasis-cloud requested a review from irisSong January 20, 2025 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants