From 3bdbe965aed5b12ed4d6f67f0baa9e427a468434 Mon Sep 17 00:00:00 2001 From: Orr Gottlieb Date: Fri, 22 May 2026 00:02:59 +0100 Subject: [PATCH 1/2] perf(core): memoize hot leaf components and lazy-render Tooltip content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wraps Button, Avatar, AttentionBox, and Dialog in `React.memo` so they no longer re-render when their parent re-renders with the same props. These leaf components are rendered hundreds to thousands of times per page in monday.com, so cycles spent on no-op reconciliation compound. Tooltip is a class PureComponent, but `renderTooltipContent()` was running on every render even when the tooltip wasn't shown — a page with hundreds of avatars + tooltips paid that cost on every keystroke in unrelated components. Now `renderTooltipContent` short-circuits unless the tooltip is actually visible (gated by `state.shown` and `open`/`withoutDialog` props). `displayName` is preserved on every wrapped component so React DevTools still shows "Button" rather than "Memo(...)". `// TODO(perf):` comments flag the consumer call sites that still pass inline closures/objects — fixing those is out of scope here. Audit finding #5. --- .../components/button/src/Button/Button.tsx | 11 +++++++++-- .../components/dialog/src/Dialog/Dialog.tsx | 11 +++++++++-- .../components/tooltip/src/Tooltip/Tooltip.tsx | 18 ++++++++++++++++-- .../components/AttentionBox/AttentionBox.tsx | 11 +++++++++-- packages/core/src/components/Avatar/Avatar.tsx | 11 +++++++++-- 5 files changed, 52 insertions(+), 10 deletions(-) diff --git a/packages/components/button/src/Button/Button.tsx b/packages/components/button/src/Button/Button.tsx index 8d86b98f52..0602be7ed6 100644 --- a/packages/components/button/src/Button/Button.tsx +++ b/packages/components/button/src/Button/Button.tsx @@ -1,5 +1,5 @@ /* eslint-disable react/button-has-type */ -import React, { type AriaAttributes, forwardRef, useCallback, useEffect, useMemo, useRef } from "react"; +import React, { type AriaAttributes, forwardRef, memo, useCallback, useEffect, useMemo, useRef } from "react"; import { camelCase } from "es-toolkit"; import cx from "classnames"; import { useMergeRef, NOOP } from "@vibe/shared"; @@ -93,7 +93,9 @@ export interface ButtonProps extends VibeComponentProps { tabIndex?: number; } -const Button = forwardRef( +// TODO(perf): callers commonly pass inline `onClick`/`children` closures which defeat memoization. +// Memoization here still helps when parent props are stable; consumer call sites are out of scope for this PR. +const ButtonComponent = forwardRef( ( { className, @@ -379,4 +381,9 @@ const Button = forwardRef( } ); +ButtonComponent.displayName = "Button"; + +const Button = memo(ButtonComponent); +Button.displayName = "Button"; + export default Button; diff --git a/packages/components/dialog/src/Dialog/Dialog.tsx b/packages/components/dialog/src/Dialog/Dialog.tsx index f3bda28a99..a5a4c6a27c 100644 --- a/packages/components/dialog/src/Dialog/Dialog.tsx +++ b/packages/components/dialog/src/Dialog/Dialog.tsx @@ -1,5 +1,5 @@ import cx from "classnames"; -import React, { useState, useEffect, useRef, useContext, useCallback, useMemo } from "react"; +import React, { memo, useState, useEffect, useRef, useContext, useCallback, useMemo } from "react"; import { createPortal } from "react-dom"; import { useFloating, @@ -29,7 +29,9 @@ import styles from "./Dialog.module.scss"; import { type DialogTriggerEvent, type DialogEvent, type DialogProps } from "./Dialog.types"; import { LayerContext, LayerProvider } from "@vibe/layer"; -function Dialog({ +// TODO(perf): consumers commonly pass inline `content` functions/`children`/`middleware` arrays which defeat +// memoization. Fixing those is out of scope for this PR; memo still avoids re-renders when parents pass stable props. +function DialogComponent({ // Core props id, "data-testid": dataTestId, @@ -545,4 +547,9 @@ function Dialog({ ); } +DialogComponent.displayName = "Dialog"; + +const Dialog = memo(DialogComponent); +Dialog.displayName = "Dialog"; + export default Dialog; diff --git a/packages/components/tooltip/src/Tooltip/Tooltip.tsx b/packages/components/tooltip/src/Tooltip/Tooltip.tsx index 0aa20035bd..56e5a394e5 100644 --- a/packages/components/tooltip/src/Tooltip/Tooltip.tsx +++ b/packages/components/tooltip/src/Tooltip/Tooltip.tsx @@ -113,7 +113,13 @@ const globalState: { lastTooltipHideTS: number; openTooltipsCount: number } = { openTooltipsCount: 0 }; -export default class Tooltip extends PureComponent { +interface TooltipState { + shown: boolean; +} + +// TODO(perf): consumers commonly pass inline `content` (functions/elements) which can't be deduped here. +// Lazy content rendering below avoids the work when the tooltip isn't visible. +export default class Tooltip extends PureComponent { wasShown: boolean; /* eslint-disable react/default-props-match-prop-types -- props inherited from DialogProps via Omit<> */ static defaultProps = { @@ -143,10 +149,16 @@ export default class Tooltip extends PureComponent { this.onTooltipHide = this.onTooltipHide.bind(this); this.wasShown = false; + this.state = { shown: !!props.shouldShowOnMount }; } renderTooltipContent() { - const { theme, content, className, style, maxWidth, title, image, icon, dir } = this.props; + const { theme, content, className, style, maxWidth, title, image, icon, dir, withoutDialog, open } = this.props; + // Lazy content rendering: skip the (potentially expensive) tooltip body unless the tooltip is + // actually visible. Dialog still calls this on every render, but we short-circuit early when hidden. + if (!withoutDialog && !this.state.shown && !open) { + return null; + } if (!content) { // don't render empty tooltip return null; @@ -192,6 +204,7 @@ export default class Tooltip extends PureComponent { const { onTooltipShow } = this.props; globalState.openTooltipsCount++; this.wasShown = true; + this.setState({ shown: true }); onTooltipShow && onTooltipShow(); } } @@ -202,6 +215,7 @@ export default class Tooltip extends PureComponent { globalState.lastTooltipHideTS = Date.now(); globalState.openTooltipsCount--; this.wasShown = false; + this.setState({ shown: false }); onTooltipHide && onTooltipHide(); } } diff --git a/packages/core/src/components/AttentionBox/AttentionBox.tsx b/packages/core/src/components/AttentionBox/AttentionBox.tsx index c6edae9356..f6deebfe2f 100644 --- a/packages/core/src/components/AttentionBox/AttentionBox.tsx +++ b/packages/core/src/components/AttentionBox/AttentionBox.tsx @@ -1,4 +1,4 @@ -import React, { forwardRef } from "react"; +import React, { forwardRef, memo } from "react"; import cx from "classnames"; import type { AttentionBoxProps, AttentionBoxRole } from "./AttentionBox.types"; import AttentionBoxDefault from "./layouts/AttentionBoxDefault/AttentionBoxDefault"; @@ -8,7 +8,9 @@ import { ComponentDefaultTestId, ComponentVibeId } from "../../tests/constants"; import { getTestId } from "../../tests/test-ids-utils"; import styles from "./AttentionBox.module.scss"; -const AttentionBox = forwardRef( +// TODO(perf): callers may pass inline `action`/`link` objects which defeat memoization. +// Memoization helps when parents pass stable props; fixing call sites is out of scope. +const AttentionBoxComponent = forwardRef( ( { compact = false, @@ -71,4 +73,9 @@ const AttentionBox = forwardRef( } ); +AttentionBoxComponent.displayName = "AttentionBox"; + +const AttentionBox = memo(AttentionBoxComponent); +AttentionBox.displayName = "AttentionBox"; + export default AttentionBox; diff --git a/packages/core/src/components/Avatar/Avatar.tsx b/packages/core/src/components/Avatar/Avatar.tsx index 9df1cd92a3..4c4c93cb19 100644 --- a/packages/core/src/components/Avatar/Avatar.tsx +++ b/packages/core/src/components/Avatar/Avatar.tsx @@ -2,7 +2,7 @@ import { camelCase } from "es-toolkit"; import { getStyle } from "@vibe/shared"; import { ComponentDefaultTestId, getTestId } from "../../tests/test-ids-utils"; import cx from "classnames"; -import React, { type AriaRole, useCallback, useMemo } from "react"; +import React, { type AriaRole, memo, useCallback, useMemo } from "react"; import { isNil } from "es-toolkit"; import { type ElementAllowedColor, getElementColor } from "../../types/Colors"; import { type AvatarSize, type AvatarType } from "./Avatar.types"; @@ -114,7 +114,9 @@ export interface AvatarProps extends VibeComponentProps { onClick?: (event: React.MouseEvent | React.KeyboardEvent, avatarId: string) => void; } -const Avatar = ({ +// TODO(perf): consumers may pass inline `tooltipProps`/badge prop objects, defeating memoization at call sites. +// Fixing those is out of scope; memo still avoids re-renders when parents pass stable props. +const AvatarComponent = ({ id, type = "text", className, @@ -264,4 +266,9 @@ const Avatar = ({ ); }; +AvatarComponent.displayName = "Avatar"; + +const Avatar = memo(AvatarComponent); +Avatar.displayName = "Avatar"; + export default Avatar; From 80b030ab39bb0997d1f86ded4900a1dd67848758 Mon Sep 17 00:00:00 2001 From: Orr Gottlieb Date: Fri, 22 May 2026 08:27:15 +0100 Subject: [PATCH 2/2] fix(tooltip): revert lazy content render that broke Dialog setup Returning null from renderTooltipContent makes Dialog short-circuit to just render children (no triggers attached), so the tooltip never opens. Combined with Dialog now being memoized, Tooltip state changes also don't propagate to force a re-render of content. Keep the leaf component memoizations (Button, Avatar, AttentionBox, Dialog) which are the main perf wins. Co-Authored-By: Claude Opus 4.7 --- .../components/tooltip/src/Tooltip/Tooltip.tsx | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/packages/components/tooltip/src/Tooltip/Tooltip.tsx b/packages/components/tooltip/src/Tooltip/Tooltip.tsx index 56e5a394e5..0aa20035bd 100644 --- a/packages/components/tooltip/src/Tooltip/Tooltip.tsx +++ b/packages/components/tooltip/src/Tooltip/Tooltip.tsx @@ -113,13 +113,7 @@ const globalState: { lastTooltipHideTS: number; openTooltipsCount: number } = { openTooltipsCount: 0 }; -interface TooltipState { - shown: boolean; -} - -// TODO(perf): consumers commonly pass inline `content` (functions/elements) which can't be deduped here. -// Lazy content rendering below avoids the work when the tooltip isn't visible. -export default class Tooltip extends PureComponent { +export default class Tooltip extends PureComponent { wasShown: boolean; /* eslint-disable react/default-props-match-prop-types -- props inherited from DialogProps via Omit<> */ static defaultProps = { @@ -149,16 +143,10 @@ export default class Tooltip extends PureComponent { this.onTooltipHide = this.onTooltipHide.bind(this); this.wasShown = false; - this.state = { shown: !!props.shouldShowOnMount }; } renderTooltipContent() { - const { theme, content, className, style, maxWidth, title, image, icon, dir, withoutDialog, open } = this.props; - // Lazy content rendering: skip the (potentially expensive) tooltip body unless the tooltip is - // actually visible. Dialog still calls this on every render, but we short-circuit early when hidden. - if (!withoutDialog && !this.state.shown && !open) { - return null; - } + const { theme, content, className, style, maxWidth, title, image, icon, dir } = this.props; if (!content) { // don't render empty tooltip return null; @@ -204,7 +192,6 @@ export default class Tooltip extends PureComponent { const { onTooltipShow } = this.props; globalState.openTooltipsCount++; this.wasShown = true; - this.setState({ shown: true }); onTooltipShow && onTooltipShow(); } } @@ -215,7 +202,6 @@ export default class Tooltip extends PureComponent { globalState.lastTooltipHideTS = Date.now(); globalState.openTooltipsCount--; this.wasShown = false; - this.setState({ shown: false }); onTooltipHide && onTooltipHide(); } }