fix: detect iframe DOM nodes#768
Conversation
|
@PinkChampagne17 is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
漫步该 PR 修改 DOM 节点检测机制，将基于同源构造函数的 变更iframe 跨文档 DOM 识别
sequenceDiagram
participant Test
participant findDOMNode
participant isDOM
participant isVisible
participant iframeElem as iframe 内元素
Test->>iframeElem: 在 iframe 中创建 div 并发起测试调用
Test->>findDOMNode: 调用 findDOMNode/getDOM with node
findDOMNode->>isDOM: 调用 isDOM(node)
isDOM->>iframeElem: 检查 nodeType === 1
iframeElem-->>isDOM: true
isDOM-->>findDOMNode: true
findDOMNode-->>Test: 返回 iframe 内 div
Test->>isVisible: 调用 isVisible(node)
isVisible->>iframeElem: 检查 nodeType === 1
isVisible->>iframeElem: 调用 getBoundingClientRect（被 mock）
iframeElem-->>isVisible: 返回尺寸数据
isVisible-->>Test: 返回 true
预估代码审查工作量🎯 2 (Simple) | ⏱️ ~12 分钟 可能相关的 PR
建议的审查者
诗
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/Dom/findDOMNode.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/Dom/isVisible.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. tests/findDOMNode.test.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
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 |
![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 updates the DOM element checks in findDOMNode and isVisible to use nodeType === 1 instead of instanceof checks, ensuring cross-frame compatibility (e.g., with iframes). Corresponding unit tests have been added to verify this behavior. A review comment suggests making the isDOM check more robust by explicitly verifying that the node is an object before checking its nodeType property.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Dom/isVisible.ts (1)
6-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick win加强
isVisible的运行时输入校验，避免nodeType伪对象误判
src/Dom/isVisible.ts入口目前只用element.nodeType === 1来放行后续的offsetParent/getBBox/getBoundingClientRect判断；如果运行时传入的不是单纯的真实Element（哪怕具备nodeType: 1并挂了同名方法/字段），就可能被误判为可见。建议在入口加入更稳的“真实 Element 特征”校验（例如nodeName等），与src/Dom/findDOMNode.ts里“跨 frame 不能用instanceof”的做法保持一致但仍收紧运行时契约。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Dom/isVisible.ts` around lines 6 - 24, The isVisible function currently trusts element.nodeType === 1 alone, which allows pseudo-objects with the same property names to slip through; tighten the runtime check before using offsetParent/getBBox/getBoundingClientRect by ensuring the value is a real DOM element — e.g., verify typeof element === 'object' && element !== null && element.nodeType === 1 && typeof element.nodeName === 'string' and/or presence of ownerDocument/documentElement properties (mirroring the safer, frame-crossing-safe checks used in findDOMNode.ts) so only genuine Elements reach the offsetParent/getBBox/getBoundingClientRect branches; update isVisible to perform this gate check and early-return false for invalid inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Dom/findDOMNode.ts`:
- Around line 3-7: isDOM currently only checks nodeType===1 and can misclassify
plain objects like {nodeType:1}; update isDOM to more robustly detect real DOM
elements by ensuring node is a non-null object, nodeType===1, and at least one
real DOM property exists (e.g. typeof node.nodeName === "string" and typeof
(node as any).ownerDocument === "object") or when available use the safe
cross-frame pattern (try checking typeof Element !== "undefined" && node
instanceof Element). Adjust the type predicate to a narrower Element (instead of
HTMLElement|SVGElement) so getDOM and findDOMNode return a true DOM Element and
callers won't be handed plain objects; update getDOM/findDOMNode uses
accordingly.
---
Outside diff comments:
In `@src/Dom/isVisible.ts`:
- Around line 6-24: The isVisible function currently trusts element.nodeType ===
1 alone, which allows pseudo-objects with the same property names to slip
through; tighten the runtime check before using
offsetParent/getBBox/getBoundingClientRect by ensuring the value is a real DOM
element — e.g., verify typeof element === 'object' && element !== null &&
element.nodeType === 1 && typeof element.nodeName === 'string' and/or presence
of ownerDocument/documentElement properties (mirroring the safer,
frame-crossing-safe checks used in findDOMNode.ts) so only genuine Elements
reach the offsetParent/getBBox/getBoundingClientRect branches; update isVisible
to perform this gate check and early-return false for invalid inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 400f1c01-9c70-4ec5-9f3b-aadbcf18ceb4
📒 Files selected for processing (4)
src/Dom/findDOMNode.tssrc/Dom/isVisible.tstests/findDOMNode.test.tsxtests/isVisible.test.ts
Co-authored-by: Codex <codex@openai.com>
PinkChampagne17
force-pushed
the
fix-iframe-dom-nodes
branch
from
826453f to
7750d4d
Compare
1 participant
Fix #460.
Summary by CodeRabbit
发布说明
Bug Fixes
Tests