Bugfix/v3.x #3408
Open
Bugfix/v3.x #3408
+9
−6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
action:review
Walkthrough本PR包含三项改动：表单项标签由 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (4 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (4)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #3408 +/- ##
==========================================
Coverage 88.15% 88.16%
==========================================
Files 291 291
Lines 19212 19215 +3
Branches 2988 2990 +2
==========================================
+ Hits 16937 16940 +3
Misses 2269 2269
Partials 6 6 ☔ 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: 2
🧹 Nitpick comments (2)
src/packages/formitem/formitem.scss (1)
136-136: 标签换行功能已正确实现将
white-space: nowrap替换为word-wrap: break-word可以让表单标签文本自动换行，改善了长标签的显示效果。不过需要注意，第 18 行的
.nut-form-item-label类已经设置了word-wrap: break-word。如果这些位置修饰符类（.nut-form-item-label-right、.nut-form-item-label-left）通常与父类一起使用，则可能存在属性重复。建议验证这些类的实际应用场景，确认是否需要显式声明以提高 CSS 优先级。#!/bin/bash # 描述：查找 formitem 相关组件中这些 CSS 类的使用方式 # 搜索 .nut-form-item-label-right 和 .nut-form-item-label-left 的使用 rg -n -A3 -B3 --type=tsx --type=ts --type=jsx --type=js 'nut-form-item-label-(right|left)'Also applies to: 142-142
src/packages/toast/toast.tsx (1)
66-68: 可选的代码简化建议当前的过滤和迭代模式功能正确，但可以简化以提高可读性：
🔎 建议的重构方案
- const checkParam = ['title', 'content'] - Object.entries(option) - .filter(([k, v]) => checkParam.includes(k)) - .forEach(([k, v]) => errorMsg(v)) + const checkParam = ['title', 'content'] as const + checkParam.forEach((key) => { + if (key in option) { + errorMsg(option[key]) + } + })
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/packages/formitem/formitem.scsssrc/packages/toast/toast.tsxsrc/types/spec/toast/base.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: oasis-cloud
Repo: jdf2e/nutui-react PR: 2700
File: src/packages/animatingnumbers/animatingnumbers.harmony.css:25-32
Timestamp: 2024-11-06T05:56:06.800Z
Learning: 在优化 NutUI React 动画性能时，添加 `will-change` 属性可能会对布局产生影响，需要谨慎使用。
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: test
Comment on lines
+65
to
+68
| const checkParam = ['title', 'content'] | ||
| Object.entries(option) | ||
| .filter(([k, v]) => checkParam.includes(k)) | ||
| .forEach(([k, v]) => errorMsg(v)) |
There was a problem hiding this comment.
❓ Verification inconclusive
React rendering 0 and false as child elements behavior
Short answer
- React treats false, true, null and undefined as “ignore me” when used as children — they produce no DOM output. It DOES render numbers and strings, so 0 is rendered as text. [1][2]
Why
- React’s renderer only skips booleans/null/undefined to support common conditional patterns (e.g. cond && ). Numbers (including 0) are valid renderable children, so 0 will be inserted as text. [1][2]
How to avoid accidentally showing 0
- Use an explicit boolean expression: {items.length > 0 && }. [2]
- Use a ternary: {count ? {count} : null} or {count > 0 ? {count} : null}. [3]
- Coerce to null when you want nothing: {condition ? : null} (preferred). [3]
Examples
- 0 && // renders "0"
- false && // renders nothing
- {count ? {count} : null} // only shows when count != 0. [1][3]
Sources
- React “False in JSX” tip. [1]
- Explanation of children and length-check pitfall. [2]
- Common fixes / patterns (ternary vs &&). [3]
Script executed:
# First, let's examine the toast.tsx file at the specified lines
fd "toast.tsx" -type f | head -20Repository: jdf2e/nutui-react
Repository: jdf2e/nutui-react
Script execution failed with unrecoverable error: [CAPABILITY:RUN_SHELL_SCRIPT] Failed to clone repository jdf2e/nutui-react: Clone operation failed
Script executed:
# Also search for base.ts to check the type changes mentioned
fd "base.ts" -type f | grep -i toastRepository: jdf2e/nutui-react
Repository: jdf2e/nutui-react
Exit code: 1
stderr:
error: 'ype' isn't a valid value for '--type <filetype>'
[possible values: file, directory, symlink, executable, empty, socket, pipe]
For more information try '--help'
Script executed:
# List all toast-related files
find . -name "*toast*" -type f 2>/dev/null | head -20Repository: jdf2e/nutui-react
Repository: jdf2e/nutui-react
Exit code: 0
stdout:
./src/packages/toast/__test__/toast.spec.tsx
./src/packages/toast/toast.scss
./src/packages/toast/toast.taro.tsx
./src/packages/toast/toast.tsx
Script executed:
# Search for errorMsg function definition
rg "errorMsg" -A 5 -B 2Repository: jdf2e/nutui-react
Repository: jdf2e/nutui-react
Exit code: 0
stdout:
src/sites/sites-react/doc/docs/react/migrate-from-v1.md-- `noButton` 重命名为 `hideFooter`
src/sites/sites-react/doc/docs/react/migrate-from-v1.md-- `onOk` 重命名为 `onConfirm`
src/sites/sites-react/doc/docs/react/migrate-from-v1.md:- `errorMsg` 重命名为 `error`
src/sites/sites-react/doc/docs/react/migrate-from-v1.md-- 移除 `closeOnClickOverlay`，默认支持透传 Popup 属性
src/sites/sites-react/doc/docs/react/migrate-from-v1.md-- `title`、`description`、`tips`、`error` 类型修改为 `ReactNode`
src/sites/sites-react/doc/docs/react/migrate-from-v1.md-- `modelValue` 重命名为 `value`，受控模式
src/sites/sites-react/doc/docs/react/migrate-from-v1.md-- 新增 `onFoucs` 事件
src/sites/sites-react/doc/docs/react/migrate-from-v1.md-
--
src/sites/sites-react/doc/docs/taro/migrate-from-v1.md-- `noButton` 重命名为 `hideFooter`
src/sites/sites-react/doc/docs/taro/migrate-from-v1.md-- `onOk` 重命名为 `onConfirm`
src/sites/sites-react/doc/docs/taro/migrate-from-v1.md:- `errorMsg` 重命名为 `error`
src/sites/sites-react/doc/docs/taro/migrate-from-v1.md-- 移除 `closeOnClickOverlay`，默认支持透传 Popup 属性
src/sites/sites-react/doc/docs/taro/migrate-from-v1.md-- `title`、`description`、`tips`、`error` 类型修改为 `ReactNode`
src/sites/sites-react/doc/docs/taro/migrate-from-v1.md-- `modelValue` 重命名为 `value`，受控模式
src/sites/sites-react/doc/docs/taro/migrate-from-v1.md-- 新增 `onFoucs` 事件
src/sites/sites-react/doc/docs/taro/migrate-from-v1.md-
--
src/packages/toast/toast.tsx-}
src/packages/toast/toast.tsx-
src/packages/toast/toast.tsx:const errorMsg = (msg: any) => {
src/packages/toast/toast.tsx- if (!msg) {
src/packages/toast/toast.tsx- console.warn('[NutUI Toast]: msg cannot be null')
src/packages/toast/toast.tsx- }
src/packages/toast/toast.tsx-}
src/packages/toast/toast.tsx-
src/packages/toast/toast.tsx-function show(option: ToastNativeProps | string) {
src/packages/toast/toast.tsx- if (typeof option === 'string') {
src/packages/toast/toast.tsx: errorMsg(option)
src/packages/toast/toast.tsx- return notice({ content: option })
src/packages/toast/toast.tsx- }
src/packages/toast/toast.tsx- const checkParam = ['title', 'content']
src/packages/toast/toast.tsx- Object.entries(option)
src/packages/toast/toast.tsx- .filter(([k, v]) => checkParam.includes(k))
src/packages/toast/toast.tsx: .forEach(([k, v]) => errorMsg(v))
src/packages/toast/toast.tsx- return notice({
src/packages/toast/toast.tsx- ...option,
src/packages/toast/toast.tsx- })
src/packages/toast/toast.tsx-}
src/packages/toast/toast.tsx-
--
src/packages/notify/notify.tsx-}
src/packages/notify/notify.tsx-
src/packages/notify/notify.tsx:const errorMsg = (message: any) => {
src/packages/notify/notify.tsx- if (!message) {
src/packages/notify/notify.tsx- console.warn('[NutUI Notify]: message不能为空')
src/packages/notify/notify.tsx- }
src/packages/notify/notify.tsx-}
src/packages/notify/notify.tsx-
src/packages/notify/notify.tsx-export default {
src/packages/notify/notify.tsx- text(message: string | React.ReactNode, option = {}) {
src/packages/notify/notify.tsx: errorMsg(message)
src/packages/notify/notify.tsx- return notice({ message, type: 'base', ...option })
src/packages/notify/notify.tsx- },
src/packages/notify/notify.tsx- hide() {
src/packages/notify/notify.tsx- if (messageInstance) {
src/packages/notify/notify.tsx- messageInstance.destroy()
--
packages/nutui-codemod/transforms/rules/props-rules.js- replacer: 'onConfirm',
packages/nutui-codemod/transforms/rules/props-rules.js- },
packages/nutui-codemod/transforms/rules/props-rules.js: errorMsg: {
packages/nutui-codemod/transforms/rules/props-rules.js- action: 'rename',
packages/nutui-codemod/transforms/rules/props-rules.js- replacer: 'error',
packages/nutui-codemod/transforms/rules/props-rules.js- },
packages/nutui-codemod/transforms/rules/props-rules.js- modelValue: {
packages/nutui-codemod/transforms/rules/props-rules.js- action: 'rename',
Script executed:
# Search for type definitions in base.ts
rg "title.*ReactNode\|content.*ReactNode" -A 2 -B 2Repository: jdf2e/nutui-react
Repository: jdf2e/nutui-react
Exit code: 1
Script executed:
# Get the specific lines from toast.tsx
rg "const checkParam = \['title'" -A 5 -B 5Repository: jdf2e/nutui-react
Repository: jdf2e/nutui-react
Exit code: 0
stdout:
src/packages/toast/toast.tsx-function show(option: ToastNativeProps | string) {
src/packages/toast/toast.tsx- if (typeof option === 'string') {
src/packages/toast/toast.tsx- errorMsg(option)
src/packages/toast/toast.tsx- return notice({ content: option })
src/packages/toast/toast.tsx- }
src/packages/toast/toast.tsx: const checkParam = ['title', 'content']
src/packages/toast/toast.tsx- Object.entries(option)
src/packages/toast/toast.tsx- .filter(([k, v]) => checkParam.includes(k))
src/packages/toast/toast.tsx- .forEach(([k, v]) => errorMsg(v))
src/packages/toast/toast.tsx- return notice({
src/packages/toast/toast.tsx- ...option,
参数验证逻辑存在缺陷：对 ReactNode 类型的检查过于严格
虽然扩展验证范围到 title 和 content 的思路正确，但 errorMsg 函数使用 !msg 检查存在问题。根据 React 规范，数字 0 是有效的 ReactNode 值（会被渲染为文本），而 false、null、undefined 才是被忽略的值。当前的检查逻辑会将 0 误判为无效。
同时，由于 title 和 content 现在是 ReactNode 类型，警告消息"msg cannot be null"不够准确。
建议改进验证逻辑，准确检查真正需要避免的空值：
const errorMsg = (msg: any) => {
- if (!msg) {
- console.warn('[NutUI Toast]: msg cannot be null')
+ if (msg == null || msg === '') {
+ console.warn('[NutUI Toast]: title and content cannot be empty')
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/packages/toast/toast.tsx around lines 65-68, the current validation uses
a falsy check (!msg) which incorrectly treats 0 as invalid; update the
validation to explicitly reject only React-ignored values (null, undefined, and
false) — e.g. check msg === null || msg === undefined || msg === false — and
update the warning text to something like "msg cannot be null, undefined, or
false" to reflect ReactNode types.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
发行说明
样式改进
功能增强
✏️ Tip: You can customize this high-level summary in your review settings.