isDom判断逻辑兼容iframe的node #639
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough这个 PR 修改了 Changes
Sequence Diagram(s)sequenceDiagram
participant T as 测试用例 (iframe)
participant I as iframe
participant S as SVG 元素
participant F as isDOM 函数
T->>I: 创建并附加 iframe 到文档
I->>I: 在 iframe 内创建 SVG 元素 (S)
T->>F: 调用 isDOM(S)
F->>S: 获取 S.ownerDocument.defaultView (win)
F-->>T: 返回类型判断结果
Suggested reviewers
Poem
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
tests/findDOMNode.test.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.js » /node_modules/.pnpm/@umijs[email protected]/node_modules/@umijs/fabric/dist/eslint.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. src/Dom/findDOMNode.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.js » /node_modules/.pnpm/@umijs[email protected]/node_modules/@umijs/fabric/dist/eslint.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
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:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/findDOMNode.test.tsx (2)
88-88: 移除调试代码请移除代码中的
debugger;语句。这类调试语句不应保留在最终的代码中。- // debugger;
80-95: 添加iframe清理代码测试创建了iframe并添加到document.body中，但没有在测试结束后将其移除。应当在测试结束时清理这些DOM元素，以避免内存泄漏和对其他测试的影响。
it('isDOM type in iframe', () => { const iframe = document.createElement('iframe'); document.body.appendChild(iframe); const svg: any = iframe.contentWindow.document.createElementNS( 'http://www.w3.org/2000/svg', 'svg', ); // This `getBoundingClientRect` is used for ts type check if (isDOM(svg) && svg.getBoundingClientRect()) { expect(true).toBeTruthy(); } else { expect(true).toBeFalsy(); } + // Clean up the iframe after test + document.body.removeChild(iframe); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Dom/findDOMNode.ts(1 hunks)tests/findDOMNode.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
tests/findDOMNode.test.tsx (1)
src/Dom/findDOMNode.ts (1)
isDOM(3-9)
🔇 Additional comments (3)
src/Dom/findDOMNode.ts (2)
4-5: 引入win变量增强了跨文档上下文的DOM节点识别能力通过获取节点所属文档的
defaultView，代码现在能够在不同的窗口上下文（如iframe中）正确识别DOM元素。这种方法比使用全局window更加健壮，可以解决在iframe环境中DOM元素无法被正确识别的问题。
8-8: 正确使用节点上下文的构造函数进行类型检查修改后的代码使用
win.HTMLElement和win.SVGElement进行类型检查，而不是直接使用全局窗口的构造函数。这解决了当节点来自不同窗口上下文（如iframe）时的类型识别问题。tests/findDOMNode.test.tsx (1)
80-95: 有效验证了在iframe环境中的isDOM功能这个测试用例很好地验证了修复的功能，确保
isDOM函数能够正确识别来自iframe环境中的DOM元素。测试逻辑清晰，与现有的DOM测试保持一致的风格。
在用
@rc-components/trigger时发现在documentPictureInPicture里的node有问题，改了这个就好了Summary by CodeRabbit
重构
测试