From 0d9c4810f433e477359864ba6e8a460742967a4d Mon Sep 17 00:00:00 2001 From: M-ZubairAhmed Date: Wed, 2 Jul 2025 22:30:17 +0530 Subject: [PATCH] [MM-64618] ResizeObserver callback fires after component unmount during channel switching (#32668) --- webapp/channels/package.json | 10 +- .../dynamic_virtualized_list/index.jsx | 2 +- .../list_item.test.tsx | 97 +++++++++++++++++++ .../{item_row_shared.tsx => list_item.tsx} | 17 ++-- ...observer.ts => list_item_size_observer.ts} | 0 5 files changed, 114 insertions(+), 12 deletions(-) create mode 100644 webapp/channels/src/components/dynamic_virtualized_list/list_item.test.tsx rename webapp/channels/src/components/dynamic_virtualized_list/{item_row_shared.tsx => list_item.tsx} (86%) rename webapp/channels/src/components/dynamic_virtualized_list/{item_row_size_observer.ts => list_item_size_observer.ts} (100%) diff --git a/webapp/channels/package.json b/webapp/channels/package.json index 2cf96bf704..487644d613 100644 --- a/webapp/channels/package.json +++ b/webapp/channels/package.json @@ -188,11 +188,11 @@ "build": "cross-env NODE_ENV=production webpack", "run": "webpack --progress --watch", "dev-server": "webpack serve --mode development", - "test": "cross-env TZ=Etc/UTC jest", - "test:watch": "cross-env TZ=Etc/UTC jest --watch", - "test:updatesnapshot": "cross-env TZ=Etc/UTC jest --updateSnapshot", - "test:debug": "cross-env TZ=Etc/UTC jest --forceExit --detectOpenHandles --verbose", - "test-ci": "cross-env TZ=Etc/UTC jest --ci --maxWorkers=100% --coverage", + "test": "cross-env TZ=Etc/UTC LC_ALL=en_US.UTF-8 LANG=en_US.UTF-8 jest", + "test:watch": "cross-env TZ=Etc/UTC LC_ALL=en_US.UTF-8 LANG=en_US.UTF-8 jest --watch", + "test:updatesnapshot": "cross-env TZ=Etc/UTC LC_ALL=en_US.UTF-8 LANG=en_US.UTF-8 jest --updateSnapshot", + "test:debug": "cross-env TZ=Etc/UTC LC_ALL=en_US.UTF-8 LANG=en_US.UTF-8 jest --forceExit --detectOpenHandles --verbose", + "test-ci": "cross-env TZ=Etc/UTC LC_ALL=en_US.UTF-8 LANG=en_US.UTF-8 jest --ci --maxWorkers=100% --coverage", "clean": "rm -rf dist node_modules .eslintcache .stylelintcache tsconfig.tsbuildinfo", "stats": "cross-env NODE_ENV=production webpack --profile --json > webpack_stats.json", "mmjstool": "mmjstool", diff --git a/webapp/channels/src/components/dynamic_virtualized_list/index.jsx b/webapp/channels/src/components/dynamic_virtualized_list/index.jsx index c0c0edbf4b..c583f8ed5f 100644 --- a/webapp/channels/src/components/dynamic_virtualized_list/index.jsx +++ b/webapp/channels/src/components/dynamic_virtualized_list/index.jsx @@ -7,7 +7,7 @@ import memoizeOne from 'memoize-one'; import {createElement, PureComponent} from 'react'; -import ListItem from './item_row_shared'; +import ListItem from './list_item'; const atBottomMargin = 10; diff --git a/webapp/channels/src/components/dynamic_virtualized_list/list_item.test.tsx b/webapp/channels/src/components/dynamic_virtualized_list/list_item.test.tsx new file mode 100644 index 0000000000..e64d4ae07e --- /dev/null +++ b/webapp/channels/src/components/dynamic_virtualized_list/list_item.test.tsx @@ -0,0 +1,97 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {render, screen} from '@testing-library/react'; +import React from 'react'; + +jest.mock('./list_item_size_observer', () => { + const mockObserve = jest.fn(() => jest.fn()); + const mockUnobserve = jest.fn(); + + return { + ListItemSizeObserver: { + getInstance: jest.fn(() => ({ + observe: mockObserve, + unobserve: mockUnobserve, + })), + }, + }; +}); + +jest.mock('lodash/debounce', () => { + return jest.fn((fn) => { + const debouncedFn = (...args: any[]) => fn(...args); + debouncedFn.cancel = jest.fn(); + debouncedFn.flush = jest.fn(); + return debouncedFn; + }); +}); + +import ListItem from './list_item'; + +describe('ListItem', () => { + const defaultProps = { + item:
{'Test Item Content'}
, + itemId: 'test-item-1', + index: 0, + height: 100, + width: 300, + onHeightChange: jest.fn(), + onUnmount: jest.fn(), + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('renders the item content correctly', () => { + render(); + + expect(screen.getByTestId('test-item')).toBeInTheDocument(); + expect(screen.getByText('Test Item Content')).toBeInTheDocument(); + }); + + test('applies correct attributes to the wrapper div', () => { + render(); + + const wrapper = screen.getByRole('listitem'); + expect(wrapper).toHaveClass('item_measurer'); + expect(wrapper).toHaveAttribute('role', 'listitem'); + }); + + test('calls onHeightChange on mount with initial height', () => { + const mockOnHeightChange = jest.fn(); + + Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { + configurable: true, + value: 120, + }); + + render( + , + ); + + expect(mockOnHeightChange).toHaveBeenCalledWith('test-item-1', 120, false); + }); + + test('handles zero offsetHeight gracefully', () => { + const mockOnHeightChange = jest.fn(); + + Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { + configurable: true, + value: 0, + }); + + render( + , + ); + + expect(mockOnHeightChange).toHaveBeenCalledWith('test-item-1', 0, false); + }); +}); diff --git a/webapp/channels/src/components/dynamic_virtualized_list/item_row_shared.tsx b/webapp/channels/src/components/dynamic_virtualized_list/list_item.tsx similarity index 86% rename from webapp/channels/src/components/dynamic_virtualized_list/item_row_shared.tsx rename to webapp/channels/src/components/dynamic_virtualized_list/list_item.tsx index a56b6ab246..d2514095e6 100644 --- a/webapp/channels/src/components/dynamic_virtualized_list/item_row_shared.tsx +++ b/webapp/channels/src/components/dynamic_virtualized_list/list_item.tsx @@ -5,7 +5,7 @@ import debounce from 'lodash/debounce'; import type {ReactNode} from 'react'; import React, {memo, useEffect, useRef} from 'react'; -import {ListItemSizeObserver} from './item_row_size_observer'; +import {ListItemSizeObserver} from './list_item_size_observer'; const RESIZE_DEBOUNCE_TIME = 200; // in ms @@ -50,9 +50,15 @@ const ListItem = (props: Props) => { // This effects adds the observer which calls height change callback debounced useEffect(() => { const debouncedOnHeightChange = debounce((changedHeight: number) => { + // Check if component is still mounted as it may have been + // unmounted by the time the debounced function is called + if (!rowRef.current) { + return; + } + // If width of container has changed then scroll bar position will be out of sync // so we need to force a scroll correction - const forceScrollCorrection = rowRef.current?.offsetWidth !== widthRef.current; + const forceScrollCorrection = rowRef.current.offsetWidth !== widthRef.current; heightRef.current = changedHeight; @@ -76,11 +82,10 @@ const ListItem = (props: Props) => { cleanupSizeObserver = listItemSizeObserver.observe(props.itemId, rowRef.current, itemRowSizeObserverCallback); } - // We remove the observer here from a row return () => { - if (cleanupSizeObserver) { - cleanupSizeObserver(); - } + // We remove the observer here from a row + cleanupSizeObserver?.(); + debouncedOnHeightChange?.cancel(); props.onUnmount(props.itemId, indexRef.current); }; }, [props.itemId]); diff --git a/webapp/channels/src/components/dynamic_virtualized_list/item_row_size_observer.ts b/webapp/channels/src/components/dynamic_virtualized_list/list_item_size_observer.ts similarity index 100% rename from webapp/channels/src/components/dynamic_virtualized_list/item_row_size_observer.ts rename to webapp/channels/src/components/dynamic_virtualized_list/list_item_size_observer.ts