优化popup组件 #538
优化popup组件 #538
Conversation
Walkthrough本次更改调整了弹窗组件的渲染逻辑。只有在弹窗处于打开状态时，才会渲染其子节点。新增了一个提前返回条件：当 Changes
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
src/Popup/index.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+fabric@4.0.1_postcss@8.5.3/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. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
| @@ -199,6 +200,10 @@ const Popup = React.forwardRef<HTMLDivElement, PopupProps>((props, ref) => { | |||
| miscStyle.pointerEvents = 'none'; | |||
| } | |||
|
|
|||
| if (!forceRender && !open) { | |||
There was a problem hiding this comment.
这些逻辑可能没有必要，Portal 里已经对 forceRender 做了处理。如果是非 open 且没有 forceRender 是不会渲染的
There was a problem hiding this comment.
我用修改前和修改后代码，跑了一下一次性创建2w个trigger，上面是用时对比，分别做了三次对比
看到Portal里面也有React.cloneElement这个操作
There was a problem hiding this comment.
所以损耗比较多的是来自 cloneElement 的代码是吧？感谢调试，我想想怎么链路上去优化一下，Portal 本身应该是让人无脑用的，如果上游还要优化就不是封装 Portal 的目的了
There was a problem hiding this comment.
我拉了一下火焰图（禁用 React Dev Tools），cloneElement 是少于 createElement 的：
There was a problem hiding this comment.
从 createElement 上看，rc-trigger 的 TriggerWrapper 可以省略，但是需要把所有依赖动态 ref 的组件的 getTriggerDOMNode 给收掉。我看看是否可以实现
There was a problem hiding this comment.
另外还有一点，React 的 dev 和 prod 的对 ReactElement 性能损耗是不一样的。createElement 的慢在于 dev 下会做额外的注入：
function ReactElement(
type,
key,
self,
source,
owner,
props,
debugStack,
debugTask
) {
self = props.ref;
type = {
$$typeof: REACT_ELEMENT_TYPE,
type: type,
key: key,
props: props,
_owner: owner
};
null !== (void 0 !== self ? self : null)
? Object.defineProperty(type, "ref", {
enumerable: !1,
get: elementRefGetterWithDeprecationWarning
})
: Object.defineProperty(type, "ref", { enumerable: !1, value: null });
type._store = {};
Object.defineProperty(type._store, "validated", {
configurable: !1,
enumerable: !1,
writable: !0,
value: 0
});
Object.defineProperty(type, "_debugInfo", {
configurable: !1,
enumerable: !1,
writable: !0,
value: null
});
Object.defineProperty(type, "_debugStack", {
configurable: !1,
enumerable: !1,
writable: !0,
value: debugStack
});
Object.defineProperty(type, "_debugTask", {
configurable: !1,
enumerable: !1,
writable: !0,
value: debugTask
});
Object.freeze && (Object.freeze(type.props), Object.freeze(type));
return type;
}而 prod 则不会：
function ReactElement(type, key, self, source, owner, props) {
self = props.ref;
return {
$$typeof: REACT_ELEMENT_TYPE,
type: type,
key: key,
ref: void 0 !== self ? self : null,
props: props
};
}所以对于 createElement or cloneElement 的优化主要是提升了 dev 下的体验。
There was a problem hiding this comment.
其实 createElement是必要操作，也不需要关心他们两个谁的效率更高
我的观点是，代码中尽可能的少用不用cloneElement代码，正如你说的TriggerWrapper中的cloneElement可以和外面的cloneElement操作合并成一次
There was a problem hiding this comment.
假设完成许许多多的性能优化，lim性能趋向于正无穷=<></>
在大批量的渲染面前，渲染100万个<></>和渲染100万个null肯定也是前者耗时长
所以const childNode =(open && (typeof popup === 'function' ? popup() : popup)) || null;这行代码的存在是非常有必要的
There was a problem hiding this comment.
嗯，v6 里不需要 findDOMNode，先干掉一个 #539
There was a problem hiding this comment.
#540 我调了一下，在更外层来处理把 Popup 和 ContextProvider 也省一下，帮我 CR 一下
在!forceRender && !open直接return null
Summary by CodeRabbit