|
| 1 | +# Prompt for @ably/ui Project - Expander Component Performance Optimization |
| 2 | + |
| 3 | +I need help optimizing the Expander component's performance. Following the successful Header.js optimization, Chrome DevTools Performance profiling has revealed that the Expander component is now the **#1 performance bottleneck** on our website (ably.com), causing severe CPU issues on iOS devices. |
| 4 | + |
| 5 | +## Problem Analysis |
| 6 | + |
| 7 | +With the Header.js fix deployed (v17.11.0-dev.4c4b6b55), Chrome DevTools now shows that **Expander.js causes 67-92ms of forced reflows** on the Chat and PubSub pages. This component has replaced Header as the primary cause of "phone gets hot" complaints from iOS users. |
| 8 | + |
| 9 | +### Current Implementation Issues |
| 10 | + |
| 11 | +The Expander component in `src/core/Expander.js` (around line 6:585 based on the webpack trace) is causing forced synchronous layout recalculations, likely during: |
| 12 | +- Component mount/render |
| 13 | +- User interactions (expand/collapse) |
| 14 | +- Scroll events (if height calculations are triggered) |
| 15 | +- Window resize events |
| 16 | + |
| 17 | +Suspected problematic patterns: |
| 18 | +- **getBoundingClientRect()** or similar layout queries |
| 19 | +- **offsetHeight/scrollHeight** checks during render |
| 20 | +- Height calculations in event handlers |
| 21 | +- State updates that trigger immediate layout reads |
| 22 | + |
| 23 | +### Performance Impact |
| 24 | + |
| 25 | +| Page | Expander.js Reflow Time | % of Total Reflows | |
| 26 | +|------|------------------------|-------------------| |
| 27 | +| Chat page | 67ms | 52% | |
| 28 | +| PubSub page | 92ms | 96% | |
| 29 | + |
| 30 | +**Comparison to Previous Bottleneck:** |
| 31 | +- Old Header issue: 71-193ms (now fixed ✅) |
| 32 | +- New Expander issue: 67-92ms (current blocker ⚠️) |
| 33 | + |
| 34 | +The Expander is nearly as bad as the old Header issue! |
| 35 | + |
| 36 | +## Suspected Code Pattern |
| 37 | + |
| 38 | +The Expander likely has code similar to this: |
| 39 | + |
| 40 | +```javascript |
| 41 | +const Expander = ({ children, ...props }) => { |
| 42 | + const [isExpanded, setIsExpanded] = useState(false); |
| 43 | + const contentRef = useRef(null); |
| 44 | + |
| 45 | + const handleToggle = () => { |
| 46 | + if (contentRef.current) { |
| 47 | + // FORCED REFLOW - reading layout properties |
| 48 | + const height = contentRef.current.scrollHeight; // or getBoundingClientRect() |
| 49 | + |
| 50 | + // Then applying styles based on the measurement |
| 51 | + contentRef.current.style.height = isExpanded ? '0' : `${height}px`; |
| 52 | + } |
| 53 | + setIsExpanded(!isExpanded); |
| 54 | + }; |
| 55 | + |
| 56 | + // Or possibly measuring on every render |
| 57 | + useEffect(() => { |
| 58 | + if (contentRef.current) { |
| 59 | + const height = contentRef.current.offsetHeight; // FORCED REFLOW |
| 60 | + // ... do something with height |
| 61 | + } |
| 62 | + }); |
| 63 | + |
| 64 | + return ( |
| 65 | + <div> |
| 66 | + <button onClick={handleToggle}>Toggle</button> |
| 67 | + <div ref={contentRef} style={{ height: isExpanded ? 'auto' : 0 }}> |
| 68 | + {children} |
| 69 | + </div> |
| 70 | + </div> |
| 71 | + ); |
| 72 | +}; |
| 73 | +``` |
| 74 | + |
| 75 | +## Proposed Solution |
| 76 | + |
| 77 | +### Option 1: CSS-Only Solution (Recommended - No JS Required) |
| 78 | + |
| 79 | +Replace JavaScript height calculations with pure CSS transitions: |
| 80 | + |
| 81 | +```typescript |
| 82 | +const Expander = ({ children, isExpanded, ...props }) => { |
| 83 | + return ( |
| 84 | + <div className="expander"> |
| 85 | + <div |
| 86 | + className={cn("expander-content", { |
| 87 | + "expander-content--expanded": isExpanded |
| 88 | + })} |
| 89 | + style={{ |
| 90 | + // Use max-height with a large value instead of measuring |
| 91 | + maxHeight: isExpanded ? '2000px' : '0', |
| 92 | + transition: 'max-height 0.3s ease-in-out', |
| 93 | + overflow: 'hidden', |
| 94 | + }} |
| 95 | + > |
| 96 | + {children} |
| 97 | + </div> |
| 98 | + </div> |
| 99 | + ); |
| 100 | +}; |
| 101 | +``` |
| 102 | + |
| 103 | +**Pros:** |
| 104 | +- Zero forced reflows |
| 105 | +- GPU-accelerated with CSS transitions |
| 106 | +- Much simpler code |
| 107 | +- Better performance on all devices |
| 108 | + |
| 109 | +**Cons:** |
| 110 | +- Need to estimate max-height (can be generous, e.g., 2000px) |
| 111 | +- Transition timing may vary based on content height |
| 112 | + |
| 113 | +### Option 2: ResizeObserver with RequestAnimationFrame (If Exact Heights Needed) |
| 114 | + |
| 115 | +If you need exact heights for some reason, use ResizeObserver instead of direct measurements: |
| 116 | + |
| 117 | +```typescript |
| 118 | +const Expander = ({ children, isExpanded, ...props }) => { |
| 119 | + const contentRef = useRef<HTMLDivElement>(null); |
| 120 | + const [contentHeight, setContentHeight] = useState(0); |
| 121 | + |
| 122 | + useEffect(() => { |
| 123 | + if (!contentRef.current) return; |
| 124 | + |
| 125 | + // Use ResizeObserver to track height changes |
| 126 | + const observer = new ResizeObserver((entries) => { |
| 127 | + // Batch updates with requestAnimationFrame |
| 128 | + requestAnimationFrame(() => { |
| 129 | + const entry = entries[0]; |
| 130 | + if (entry) { |
| 131 | + setContentHeight(entry.contentRect.height); |
| 132 | + } |
| 133 | + }); |
| 134 | + }); |
| 135 | + |
| 136 | + observer.observe(contentRef.current); |
| 137 | + |
| 138 | + return () => observer.disconnect(); |
| 139 | + }, []); |
| 140 | + |
| 141 | + return ( |
| 142 | + <div className="expander"> |
| 143 | + <div |
| 144 | + ref={contentRef} |
| 145 | + style={{ |
| 146 | + height: isExpanded ? `${contentHeight}px` : '0', |
| 147 | + transition: 'height 0.3s ease-in-out', |
| 148 | + overflow: 'hidden', |
| 149 | + willChange: isExpanded ? 'height' : 'auto', // Hint to browser |
| 150 | + }} |
| 151 | + > |
| 152 | + {children} |
| 153 | + </div> |
| 154 | + </div> |
| 155 | + ); |
| 156 | +}; |
| 157 | +``` |
| 158 | + |
| 159 | +**Pros:** |
| 160 | +- Exact height measurements |
| 161 | +- No forced reflows in render/event handlers |
| 162 | +- Handles dynamic content changes |
| 163 | + |
| 164 | +**Cons:** |
| 165 | +- More complex than CSS-only solution |
| 166 | +- Still has small overhead from ResizeObserver |
| 167 | + |
| 168 | +### Option 3: CSS Grid with fr Units (Modern Approach) |
| 169 | + |
| 170 | +Use CSS Grid's `grid-template-rows` for smooth transitions: |
| 171 | + |
| 172 | +```typescript |
| 173 | +const Expander = ({ children, isExpanded, ...props }) => { |
| 174 | + return ( |
| 175 | + <div |
| 176 | + className="expander" |
| 177 | + style={{ |
| 178 | + display: 'grid', |
| 179 | + gridTemplateRows: isExpanded ? '1fr' : '0fr', |
| 180 | + transition: 'grid-template-rows 0.3s ease-in-out', |
| 181 | + }} |
| 182 | + > |
| 183 | + <div style={{ overflow: 'hidden' }}> |
| 184 | + {children} |
| 185 | + </div> |
| 186 | + </div> |
| 187 | + ); |
| 188 | +}; |
| 189 | +``` |
| 190 | + |
| 191 | +**Pros:** |
| 192 | +- No JavaScript measurements needed |
| 193 | +- Smooth transitions regardless of content height |
| 194 | +- Modern and elegant solution |
| 195 | +- Best performance |
| 196 | + |
| 197 | +**Cons:** |
| 198 | +- Requires CSS Grid support (widely supported now) |
| 199 | +- Slightly different visual behavior |
| 200 | + |
| 201 | +## Requirements |
| 202 | + |
| 203 | +- Must maintain backward compatibility with existing Expander API |
| 204 | +- Should work with dynamic content that changes size |
| 205 | +- Must not cause forced reflows during expand/collapse |
| 206 | +- Performance target: <5ms overhead (95% improvement from current 67-92ms) |
| 207 | +- Should respect `prefers-reduced-motion` for accessibility |
| 208 | + |
| 209 | +## Testing Criteria |
| 210 | + |
| 211 | +1. Verify no getBoundingClientRect/offsetHeight calls during expand/collapse (check Performance tab) |
| 212 | +2. Verify smooth expand/collapse animations |
| 213 | +3. Verify works with dynamically changing content |
| 214 | +4. Test on iOS Safari (the primary problem platform) |
| 215 | +5. Measure forced reflow time drops below 5ms (currently 67-92ms) |
| 216 | +6. Test with nested expanders if that's a use case |
| 217 | + |
| 218 | +## Additional Context |
| 219 | + |
| 220 | +This issue was discovered immediately after fixing the Header.js scroll performance issue. The Header fix eliminated 71-193ms of forced reflows, but now Expander has become the bottleneck. |
| 221 | + |
| 222 | +**Pages affected:** |
| 223 | +- `/chat` - Uses Expander in FeatureGrid component (67ms reflows) |
| 224 | +- `/pubsub` - Uses Expander in multiple locations (92ms reflows) |
| 225 | + |
| 226 | +The Expander optimization is now the highest priority performance fix for the iOS thermal issues. |
| 227 | + |
| 228 | +Can you help me implement one of these solutions (or a better one) in the Expander component? |
0 commit comments