[EuiInMemoryTable] getDerivedStateFromProps can incorrectly block updates

[thompsongl]

The getDerivedStateFromProps method is set up with a false prioritization of updates. For instance, if both the items reference and the search reference change, only the items update propagates to new component state simply because the update logic occurs first.

eui/src/components/basic_table/in_memory_table.tsx

Lines 244 to 290 in eab1f14

	static getDerivedStateFromProps<T>(
	nextProps: EuiInMemoryTableProps<T>,
	prevState: State<T>
	) {
	if (nextProps.items !== prevState.prevProps.items) {
	// We have new items because an external search has completed, so reset pagination state.
	return {
	prevProps: {
	...prevState.prevProps,
	items: nextProps.items,
	},
	pageIndex: 0,
	};
	}
	const { sortName, sortDirection } = getInitialSorting(
	nextProps.columns,
	nextProps.sorting
	);
	if (
	sortName !== prevState.prevProps.sortName ||
	sortDirection !== prevState.prevProps.sortDirection
	) {
	return {
	sortName,
	sortDirection,
	};
	}
	const nextQuery = nextProps.search
	? (nextProps.search as EuiSearchBarProps).query
	: '';
	const prevQuery = prevState.prevProps.search
	? (prevState.prevProps.search as EuiSearchBarProps).query
	: '';
	if (nextQuery !== prevQuery) {
	return {
	prevProps: {
	...prevState.prevProps,
	search: nextProps.search,
	},
	query: getQueryFromSearch(nextProps.search, false),
	};
	}
	return null;
	}

Ideal case is to convert EuiInMemoryTable to a functional component and take advantage of hooks. Simpler case is to not return until each update logic check has finished.

[shrey]

@thompsongl I'd like to work on this issue.

[shrey]

          static getDerivedStateFromProps<T>(
           nextProps: EuiInMemoryTableProps<T>,
           prevState: State<T>
         ) {
           let updatedPrevState = prevState;
           if (nextProps.items !== prevState.prevProps.items) {
             // We have new items because an external search has completed, so reset pagination state.
             updatedPrevState = {
               ...updatedPrevState,
               prevProps: {
                 ...prevState.prevProps,
                 items: nextProps.items,
               },
               pageIndex: 0,
             };
           }
           const { sortName, sortDirection } = getInitialSorting(
             nextProps.columns,
             nextProps.sorting
           );
           if (
             sortName !== prevState.prevProps.sortName ||
             sortDirection !== prevState.prevProps.sortDirection
           ) {
             return {
               sortName,
               sortDirection,
             };
           }

           const nextQuery = nextProps.search
             ? (nextProps.search as EuiSearchBarProps).query
             : '';
           const prevQuery = prevState.prevProps.search
             ? (prevState.prevProps.search as EuiSearchBarProps).query
             : '';

           if (nextQuery !== prevQuery) {
             updatedPrevState = {
               ...updatedPrevState,
               prevProps: {
                 ...prevState.prevProps,
                 search: nextProps.search,
               },
               query: getQueryFromSearch(nextProps.search, false),
             };
           }
           if (updatedPrevState !== prevState) {
             return updatedPrevState;
           }
           return null;
         }

@thompsongl @chandlerprall Will this work?

[chandlerprall]

That middle block, returning an object with only sortName and sortDirection, should also be changed to update the updatedPrevState value. Otherwise, the changes look right enough to test :)

[thompsongl]

Otherwise, the changes look right enough to test :)

Yep, if you'd like to open a PR we can continue to work in there

[shrey]

Okay on it