Skip to content

优化popup组件 #538

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

优化popup组件 #538

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/Popup/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ const Popup = React.forwardRef<HTMLDivElement, PopupProps>((props, ref) => {
targetHeight,
} = props;

const childNode = typeof popup === 'function' ? popup() : popup;
const childNode =
(open && (typeof popup === 'function' ? popup() : popup)) || null;

// We can not remove holder only when motion finished.
const isNodeVisible = open || keepDom;
Expand Down Expand Up @@ -199,6 +200,10 @@ const Popup = React.forwardRef<HTMLDivElement, PopupProps>((props, ref) => {
miscStyle.pointerEvents = 'none';
}

if (!forceRender && !open) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些逻辑可能没有必要，Portal 里已经对 forceRender 做了处理。如果是非 open 且没有 forceRender 是不会渲染的

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

我用修改前和修改后代码，跑了一下一次性创建2w个trigger，上面是用时对比，分别做了三次对比

看到Portal里面也有React.cloneElement这个操作

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

所以损耗比较多的是来自 cloneElement 的代码是吧？感谢调试，我想想怎么链路上去优化一下，Portal 本身应该是让人无脑用的，如果上游还要优化就不是封装 Portal 的目的了

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我拉了一下火焰图（禁用 React Dev Tools），cloneElement 是少于 createElement 的：

截屏2025-05-14 15 47 56

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

从 createElement 上看，rc-trigger 的 TriggerWrapper 可以省略，但是需要把所有依赖动态 ref 的组件的 getTriggerDOMNode 给收掉。我看看是否可以实现

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

另外还有一点，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 下的体验。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

其实 createElement是必要操作，也不需要关心他们两个谁的效率更高
我的观点是，代码中尽可能的少用不用cloneElement代码，正如你说的TriggerWrapper中的cloneElement可以和外面的cloneElement操作合并成一次

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

假设完成许许多多的性能优化，lim性能趋向于正无穷=<></>
在大批量的渲染面前，渲染100万个<></>和渲染100万个null肯定也是前者耗时长
所以const childNode =(open && (typeof popup === 'function' ? popup() : popup)) || null;这行代码的存在是非常有必要的

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

嗯，v6 里不需要 findDOMNode，先干掉一个 #539

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#540 我调了一下，在更外层来处理把 Popup 和 ContextProvider 也省一下，帮我 CR 一下

return null;
}

return (
<Portal
open={forceRender || isNodeVisible}
Expand Down