- 
                Notifications
    You must be signed in to change notification settings 
- Fork 286
feat: Ellipsis校验越界不走缓存配置 #3329
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: Ellipsis校验越界不走缓存配置 #3329
Conversation
| Walkthrough在 Taro 环境新增无缓存的跨端矩形获取函数  Changes
 Sequence Diagram(s)sequenceDiagram
  participant Ellipsis as Ellipsis.verifyEllipsis
  participant RectUtil as getRectInMultiPlatformWithoutCache
  participant Platform as Browser/WeChat(Taro)
  Ellipsis->>RectUtil: 请求元素 Rect
  alt 浏览器
    RectUtil->>Platform: 调用 getRect(element)
    Platform-->>RectUtil: 返回 Rect
  else 非浏览器(Taro)
    RectUtil->>Platform: createSelectorQuery().select(#id).boundingClientRect()
    Platform-->>RectUtil: 返回 Rect
  end
  RectUtil-->>Ellipsis: 返回 Rect(无缓存)
  Ellipsis->>Ellipsis: 基于 Rect 计算并判定省略
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
 Suggested reviewers
 Poem
 Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration: 
 You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (1)
 ⏰ 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 (1)
 ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit: 
 SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type  Other keywords and placeholders
 CodeRabbit Configuration File ( | 
| Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@            Coverage Diff             @@
##           feat_v3.x    #3329   +/-   ##
==========================================
  Coverage      88.13%   88.14%           
==========================================
  Files            290      290           
  Lines          19113    19113           
  Branches        2960     2961    +1     
==========================================
+ Hits           16846    16847    +1     
+ Misses          2262     2261    -1     
  Partials           5        5           ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
 | 
There was a problem hiding this 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 (1)
src/utils/taro/get-rect.ts (1)
63-65: 微优化:async 内无需再包一层 Promise.resolve函数已标记为
async,直接return getRect(element)即可,减少一次不必要的包装,语义更简洁。- return Promise.resolve(getRect(element)) + return getRect(element) as any
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
- src/packages/ellipsis/ellipsis.taro.tsx(2 hunks)
- src/utils/taro/get-rect.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/utils/taro/get-rect.ts (1)
src/utils/get-rect.ts (2)
inBrowser(1-1)
getRect(16-48)
src/packages/ellipsis/ellipsis.taro.tsx (1)
src/utils/taro/get-rect.ts (1)
getRectInMultiPlatformWithoutCache(58-76)
🪛 GitHub Check: lint
src/packages/ellipsis/ellipsis.taro.tsx
[failure] 5-5:
Replace ·getRectInMultiPlatform,·getRectInMultiPlatformWithoutCache· with ⏎··getRectInMultiPlatform,⏎··getRectInMultiPlatformWithoutCache,⏎
🪛 ESLint
src/packages/ellipsis/ellipsis.taro.tsx
[error] 5-5: Replace ·getRectInMultiPlatform,·getRectInMultiPlatformWithoutCache· with ⏎··getRectInMultiPlatform,⏎··getRectInMultiPlatformWithoutCache,⏎
(prettier/prettier)
⏰ 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
🔇 Additional comments (1)
src/packages/ellipsis/ellipsis.taro.tsx (1)
198-198: 验证越界改为无缓存:方向正确,符合修复目标
verifyEllipsis中改为实时 rect(不走 LRU)能避免缓存导致的边界判定失真,方向正确。同时建议将初次越界判定也改为无缓存,否则在极端情况下(缓存高度低估)可能直接走“不超出”分支,从而跳过后续的校验与收缩,导致修复不完全。可在
calcEllipse里将以下调用一并切换为无缓存版本:// 当前位置:calcEllipse 内部 // 原: const refe = await getRectInMultiPlatform(rootContain.current) // 建议: const refe = await getRectInMultiPlatformWithoutCache(rootContain.current)如确认仅“二次校验”阶段不走缓存即可覆盖问题场景,也请补充说明理由,或增加用例覆盖两种路径(初次判定/二次校验)以防回归。
| export const getRectInMultiPlatformWithoutCache = async ( | ||
| element: any, | ||
| harmonyId = '' | ||
| ): Promise<Rect> => { | ||
| if (element) { | ||
| if (inBrowser) { | ||
| return Promise.resolve(getRect(element)) | ||
| } | ||
| return new Promise((resolve, reject) => { | ||
| createSelectorQuery() | ||
| .select(`#${harmonyId || element.uid}`) | ||
| .boundingClientRect() | ||
| .exec(([rects]) => { | ||
| resolve(rects) | ||
| }) | ||
| }) | ||
| } | ||
| return Promise.resolve(makeRect(0, 0)) | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
无缓存版本需兜底返回 Rect,且去掉未使用的 reject 参数
- 当 select 未命中节点时,rects可能为undefined,当前直接resolve(rects)存在类型与运行时风险(下游读取 width/height 会报错)。建议兜底为makeRect(0, 0),以确保 Promise 始终返回 Rect。
- new Promise((resolve, reject) => { ... })中- reject未使用,易触发 lint 未使用变量告警,可移除。
-    return new Promise((resolve, reject) => {
+    return new Promise((resolve) => {
       createSelectorQuery()
         .select(`#${harmonyId || element.uid}`)
         .boundingClientRect()
         .exec(([rects]) => {
-          resolve(rects)
+          resolve(rects || makeRect(0, 0))
         })
     })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getRectInMultiPlatformWithoutCache = async ( | |
| element: any, | |
| harmonyId = '' | |
| ): Promise<Rect> => { | |
| if (element) { | |
| if (inBrowser) { | |
| return Promise.resolve(getRect(element)) | |
| } | |
| return new Promise((resolve, reject) => { | |
| createSelectorQuery() | |
| .select(`#${harmonyId || element.uid}`) | |
| .boundingClientRect() | |
| .exec(([rects]) => { | |
| resolve(rects) | |
| }) | |
| }) | |
| } | |
| return Promise.resolve(makeRect(0, 0)) | |
| } | |
| export const getRectInMultiPlatformWithoutCache = async ( | |
| element: any, | |
| harmonyId = '' | |
| ): Promise<Rect> => { | |
| if (element) { | |
| if (inBrowser) { | |
| return Promise.resolve(getRect(element)) | |
| } | |
| return new Promise((resolve) => { | |
| createSelectorQuery() | |
| .select(`#${harmonyId || element.uid}`) | |
| .boundingClientRect() | |
| .exec(([rects]) => { | |
| resolve(rects || makeRect(0, 0)) | |
| }) | |
| }) | |
| } | |
| return Promise.resolve(makeRect(0, 0)) | |
| } | 
🤖 Prompt for AI Agents
In src/utils/taro/get-rect.ts around lines 58 to 76, the Promise resolves
undefined when the selector misses a node and also declares an unused reject
parameter; change the exec callback to resolve a safe Rect fallback (e.g.
makeRect(0, 0)) when rects is falsy so the Promise always returns a Rect, and
remove the unused reject parameter from the new Promise signature to avoid lint
warnings.
There was a problem hiding this 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
♻️ Duplicate comments (1)
src/packages/ellipsis/ellipsis.taro.tsx (1)
5-8: 修复 Prettier/ESLint 报错:按行拆分并添加尾逗号的多成员 import静态检查仍然提示 prettier/prettier 报错;需要给多行 import 的最后一个成员补上尾逗号,并去除多余空格。这样也与项目既有风格保持一致。
-import { - getRectInMultiPlatform, - getRectInMultiPlatformWithoutCache -} from '@/utils/taro/get-rect' +import { + getRectInMultiPlatform, + getRectInMultiPlatformWithoutCache, +} from '@/utils/taro/get-rect'
🧹 Nitpick comments (1)
src/packages/ellipsis/ellipsis.taro.tsx (1)
160-166: 建议:首次溢出判断也使用无缓存版本,避免缓存导致的误判虽然本次修复重点是“边界校验不走缓存”,但 calcEllipse 的首次判断仍用缓存版本,可能在 Taro 环境下因缓存未更新而误判为不溢出,进而完全跳过后续 verify 流程。为保证一致性与鲁棒性,建议这里也改为无缓存并传入明确 id。
- const refe = await getRectInMultiPlatform(rootContain.current) + const refe = await getRectInMultiPlatformWithoutCache( + rootContain.current, + `rootContain${refRandomId.current}` + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
- src/packages/ellipsis/ellipsis.taro.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/packages/ellipsis/ellipsis.taro.tsx (1)
src/utils/taro/get-rect.ts (1)
getRectInMultiPlatformWithoutCache(58-76)
🪛 GitHub Check: lint
src/packages/ellipsis/ellipsis.taro.tsx
[failure] 7-7:
Replace · with ,
[failure] 6-6:
Delete ·
[failure] 5-5:
Delete ·
🪛 ESLint
src/packages/ellipsis/ellipsis.taro.tsx
[error] 5-5: Delete ·
(prettier/prettier)
[error] 6-6: Delete ·
(prettier/prettier)
[error] 7-7: Replace · with ,
(prettier/prettier)
⏰ 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: test
- GitHub Check: build
| // 验证省略号 | ||
| const verifyEllipsis = async () => { | ||
| const refe = await getRectInMultiPlatform(rootContain.current) | ||
| const refe = await getRectInMultiPlatformWithoutCache(rootContain.current) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
校验越界改为实时获取矩形方向正确,但建议显式传入节点 id 提升跨端稳定性
getRectInMultiPlatformWithoutCache 在非浏览器分支通过 createSelectorQuery().select(#${harmonyId || element.uid}) 查询;当前未传入 harmonyId,依赖 element.uid 存在且正确,跨端(如小程序/Harmony)不一定可靠。组件里已设置了明确的 id,可直接传入,避免选择器失败。
-    const refe = await getRectInMultiPlatformWithoutCache(rootContain.current)
+    const refe = await getRectInMultiPlatformWithoutCache(
+      rootContain.current,
+      `rootContain${refRandomId.current}`
+    )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const refe = await getRectInMultiPlatformWithoutCache(rootContain.current) | |
| const refe = await getRectInMultiPlatformWithoutCache( | |
| rootContain.current, | |
| `rootContain${refRandomId.current}` | |
| ) | 
🤖 Prompt for AI Agents
In src/packages/ellipsis/ellipsis.taro.tsx around line 201 the call
getRectInMultiPlatformWithoutCache(rootContain.current) relies on element.uid in
non-browser branches which is fragile across platforms; update the call to
explicitly pass the component's declared node id (the id prop you set on the
DOM/node — e.g., harmonyId or the component's id variable) so the selector uses
that id instead of uid, and adjust the call/signature typing if needed to supply
the id in the expected form (with or without '#' per helper expectations).
🤔 这个变动的性质是?
🔗 相关 Issue
#3267 (comment)
💡 需求背景和解决方案
校验是否越界,不走lru缓存,获取实时位置
☑️ 请求合并前的自查清单
Summary by CodeRabbit
Bug Fixes
Performance
New Features