Skip to content

Commit

Permalink
fix: initial message load issues (#2292)
Browse files Browse the repository at this point in the history
### 🎯 Goal

This PR covers a range of issues that originate from incorrect
initialization of message list state. All the cases are covered below.

### πŸ›  Implementation details

#### `ChannelList`, `Channel`, `MessageList`, `VirtualizedMessageList`
not respecting the message limits

We have several default limits listed in the `limit.ts` file. However
they were not always respected:

1. `MessageList`, `VirtualizedMessageList` didn't use
`DEFAULT_NEXT_CHANNEL_PAGE_SIZE` and defaulted to the hard-coded message
limit of 100.
2. `Channel` didn't use `DEFAULT_INITIAL_CHANNEL_PAGE_SIZE` when
fetching the first page of messages.
3. `ChannelList` didn't use `DEFAULT_INITIAL_CHANNEL_PAGE_SIZE` as the
message limit per channel when fetching channels.

This is now fixed.

#### False-negative `hasMore` when messages are fetched by `ChannelList`

We have two main ways to initialize the message list: it can be
initialized by the `Channel` component; or it can be initialized by the
`ChannelList` component (using `usePaginatedChannels` hook). If the
message list is initialized by the `ChannelList`, the `Channel`
component will not try to query the channel, and will use messages
fetched by the `ChannelList` instead.

The problem here is that to determine if the channel has more messages
to load, we used to compare the number of messages currently in the
channel with the limit set on the `Channel`. But since it was not the
`Channel` but the `ChannelList` that fetched the messages, this check
makes no sense: `ChannelList` has its own separate limits.

Consider this example:

```tsx
const App = () => (
  <Chat client={chatClient}>
    <ChannelList options={{ message_limit: 5 }} />
    <Channel channelQueryOptions={{ members: { limit: 10 } }}>
      {/* ... */}
    </Channel>
  </Chat>
);
```

The `Channel` component will compare the number of messages fetched by
the `ChannelList` (5) with its own limit (10) and determine that the
channel has no more messages.

This is fixed by always setting `hasMore` to false in channel if it was
not the `Channel` component that fetched the messages. In the worst case
we'll just make one redundant query.

#### The "tall window" problem

In case the message container is very tall, or the message limit for the
initial load is very small, we can run into a situation when the first
page of messages is not long enough to trigger overflow in the message
list. Without overflow, scroll events don't fire, so the next page of
messages is never fetched.

This is not usually a problem in the `VirtualizedMessageList`, since it
will immediately load the next page _once_ in this case. However, the
`MessageList` didn't have this logic in place. It is now added (by
invoking the `scrollListener` _once_ on mount).

**Note:** both `VirtualizedMessageList` and `MessageList` will only try
loading the next page _once_ if the first page of messages is not long
enough to cause overflow. A better solution would be to keep loading
pages until there's overflow or there is no more pages. This solution is
not currently possible to implement in `VirtualizedMessageList`, so for
the sake of consistency I didn't implement it `MessageList` (event
though it is possible).

In most cases this not an issue, since our default page size is 100
messages which is enough to fill even very tall containers.
  • Loading branch information
myandrienko committed Feb 23, 2024
1 parent 652e3a5 commit 3685030
Show file tree
Hide file tree
Showing 11 changed files with 61 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,10 @@ Array of allowed message actions (ex: 'edit', 'delete', 'reply'). To disable all

The limit to use when paginating new messages (the page size).

:::caution
After mounting, the `MessageList` component checks if the list is completely filled with messages. If there is some space left in the list, `MessageList` will load the next page of messages, but it will do so _only once_. This means that if your `messageLimit` is too low, or if your viewport is very large, the list will not be completely filled. Set the limit with this in mind.
:::

| Type | Default |
| ------ | ------- |
| number | 100 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ Custom UI component to display an individual message.

The limit to use when paginating messages (the page size).

:::caution
After mounting, the `VirtualizedMessageList` component checks if the list is completely filled with messages. If there is some space left in the list, `VirtualizedMessageList` will load the next page of messages, but it will do so _only once_. This means that if your `messageLimit` is too low, or if your viewport is very large, the list will not be completely filled. Set the limit with this in mind.
:::

| Type | Default |
| ------ | ------- |
| number | 100 |
Expand Down
2 changes: 1 addition & 1 deletion examples/typescript/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as serviceWorker from './serviceWorker';
createRoot(document.getElementById('root')!).render(
<React.StrictMode>
<App />
</React.StrictMode>
</React.StrictMode>,
);

// If you want your app to work offline and load faster, you can change
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
"isomorphic-ws": "^4.0.1",
"linkifyjs": "^4.1.0",
"lodash.debounce": "^4.0.8",
"lodash.defaultsdeep": "^4.6.1",
"lodash.throttle": "^4.1.1",
"lodash.uniqby": "^4.7.0",
"nanoid": "^3.3.4",
Expand Down Expand Up @@ -155,6 +156,7 @@
"@types/jsdom": "^21.1.5",
"@types/linkifyjs": "^2.1.3",
"@types/lodash.debounce": "^4.0.7",
"@types/lodash.defaultsdeep": "^4.6.9",
"@types/lodash.throttle": "^4.1.7",
"@types/lodash.uniqby": "^4.7.7",
"@types/moment": "^2.13.0",
Expand Down
25 changes: 20 additions & 5 deletions src/components/Channel/Channel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import React, {
} from 'react';

import debounce from 'lodash.debounce';
import defaultsDeep from 'lodash.defaultsdeep';
import throttle from 'lodash.throttle';
import {
ChannelAPIResponse,
Expand Down Expand Up @@ -346,7 +347,7 @@ const ChannelInner = <
acceptedFiles,
activeUnreadHandler,
channel,
channelQueryOptions,
channelQueryOptions: propChannelQueryOptions,
children,
doDeleteMessageRequest,
doMarkReadRequest,
Expand All @@ -366,6 +367,16 @@ const ChannelInner = <
skipMessageDataMemoization,
} = props;

const channelQueryOptions: ChannelQueryOptions<StreamChatGenerics> & {
messages: { limit: number };
} = useMemo(
() =>
defaultsDeep(propChannelQueryOptions, {
messages: { limit: DEFAULT_INITIAL_CHANNEL_PAGE_SIZE },
}),
[propChannelQueryOptions],
);

const {
client,
customClasses,
Expand Down Expand Up @@ -546,6 +557,7 @@ const ChannelInner = <
useLayoutEffect(() => {
let errored = false;
let done = false;
let channelInitializedExternally = true;

(async () => {
if (!channel.initialized && initializeOnMount) {
Expand All @@ -571,6 +583,7 @@ const ChannelInner = <
await getChannel({ channel, client, members, options: channelQueryOptions });
const config = channel.getConfig();
setChannelConfig(config);
channelInitializedExternally = false;
} catch (e) {
dispatch({ error: e as Error, type: 'setError' });
errored = true;
Expand All @@ -583,10 +596,12 @@ const ChannelInner = <
if (!errored) {
dispatch({
channel,
hasMore: hasMoreMessagesProbably(
channel.state.messages.length,
channelQueryOptions?.messages?.limit ?? DEFAULT_INITIAL_CHANNEL_PAGE_SIZE,
),
hasMore:
channelInitializedExternally ||
hasMoreMessagesProbably(
channel.state.messages.length,
channelQueryOptions.messages.limit,
),
type: 'initStateFromChannel',
});

Expand Down
2 changes: 1 addition & 1 deletion src/components/Channel/__tests__/Channel.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ describe('Channel', () => {

await waitFor(() => {
expect(watchSpy).toHaveBeenCalledTimes(1);
expect(watchSpy).toHaveBeenCalledWith(undefined);
expect(watchSpy).toHaveBeenCalledWith({ messages: { limit: 25 } });
});
});

Expand Down
2 changes: 2 additions & 0 deletions src/components/ChannelList/hooks/usePaginatedChannels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { useChatContext } from '../../../context/ChatContext';

import type { DefaultStreamChatGenerics } from '../../../types/types';
import type { ChannelsQueryState } from '../../Chat/hooks/useChannelsQueryState';
import { DEFAULT_INITIAL_CHANNEL_PAGE_SIZE } from '../../../constants/limits';

const RECOVER_LOADED_CHANNELS_THROTTLE_INTERVAL_IN_MS = 5000;
const MIN_RECOVER_LOADED_CHANNELS_THROTTLE_INTERVAL_IN_MS = 2000;
Expand Down Expand Up @@ -79,6 +80,7 @@ export const usePaginatedChannels = <

const newOptions = {
limit: options?.limit ?? MAX_QUERY_CHANNELS_LIMIT,
message_limit: options?.message_limit ?? DEFAULT_INITIAL_CHANNEL_PAGE_SIZE,
offset,
...options,
};
Expand Down
20 changes: 8 additions & 12 deletions src/components/InfiniteScrollPaginator/InfiniteScroll.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { PropsWithChildren, useCallback, useEffect, useLayoutEffect, useRef } from 'react';
import React, { PropsWithChildren, useEffect, useLayoutEffect, useRef } from 'react';
import type { PaginatorProps } from '../../types/types';
import { deprecationAndReplacementWarning } from '../../utils/deprecationWarning';

Expand Down Expand Up @@ -73,7 +73,8 @@ export const InfiniteScroll = (props: PropsWithChildren<InfiniteScrollProps>) =>

const scrollComponent = useRef<HTMLElement>();

const scrollListener = useCallback(() => {
const scrollListenerRef = useRef<() => void>();
scrollListenerRef.current = () => {
const element = scrollComponent.current;

if (!element || element.offsetParent === null) {
Expand Down Expand Up @@ -103,15 +104,7 @@ export const InfiniteScroll = (props: PropsWithChildren<InfiniteScrollProps>) =>
if (offset < Number(threshold) && typeof loadNextPageFn === 'function' && hasNextPageFlag) {
loadNextPageFn();
}
}, [
hasPreviousPageFlag,
hasNextPageFlag,
isLoading,
listenToScroll,
loadPreviousPageFn,
loadNextPageFn,
threshold,
]);
};

useEffect(() => {
deprecationAndReplacementWarning(
Expand All @@ -130,14 +123,17 @@ export const InfiniteScroll = (props: PropsWithChildren<InfiniteScrollProps>) =>
const scrollElement = scrollComponent.current?.parentNode;
if (!scrollElement) return;

const scrollListener = () => scrollListenerRef.current?.();

scrollElement.addEventListener('scroll', scrollListener, useCapture);
scrollElement.addEventListener('resize', scrollListener, useCapture);
scrollListener();

return () => {
scrollElement.removeEventListener('scroll', scrollListener, useCapture);
scrollElement.removeEventListener('resize', scrollListener, useCapture);
};
}, [initialLoad, scrollListener, useCapture]);
}, [initialLoad, useCapture]);

useEffect(() => {
const scrollElement = scrollComponent.current?.parentNode;
Expand Down
3 changes: 2 additions & 1 deletion src/components/MessageList/MessageList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import type { MessageProps } from '../Message/types';
import type { StreamMessage } from '../../context/ChannelStateContext';

import type { DefaultStreamChatGenerics } from '../../types/types';
import { DEFAULT_NEXT_CHANNEL_PAGE_SIZE } from '../../constants/limits';

type MessageListWithContextProps<
StreamChatGenerics extends DefaultStreamChatGenerics = DefaultStreamChatGenerics
Expand Down Expand Up @@ -68,7 +69,7 @@ const MessageListWithContext = <
headerPosition,
read,
renderMessages = defaultRenderMessages,
messageLimit = 100,
messageLimit = DEFAULT_NEXT_CHANNEL_PAGE_SIZE,
loadMore: loadMoreCallback,
loadMoreNewer: loadMoreNewerCallback,
hasMoreNewer = false,
Expand Down
7 changes: 5 additions & 2 deletions src/components/MessageList/VirtualizedMessageList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import { ComponentContextValue, useComponentContext } from '../../context/Compon

import type { Channel, ChannelState as StreamChannelState, UserResponse } from 'stream-chat';
import type { DefaultStreamChatGenerics, UnknownType } from '../../types/types';
import { DEFAULT_NEXT_CHANNEL_PAGE_SIZE } from '../../constants/limits';

type VirtualizedMessageListPropsForContext =
| 'additionalMessageInputProps'
Expand Down Expand Up @@ -184,7 +185,7 @@ const VirtualizedMessageListWithContext = <
loadMoreNewer,
Message: MessageUIComponentFromProps,
messageActions,
messageLimit = 100,
messageLimit = DEFAULT_NEXT_CHANNEL_PAGE_SIZE,
messages,
notifications,
// TODO: refactor to scrollSeekPlaceHolderConfiguration and components.ScrollSeekPlaceholder, like the Virtuoso Component
Expand Down Expand Up @@ -386,7 +387,9 @@ const VirtualizedMessageListWithContext = <
}
};
const atTopStateChange = (isAtTop: boolean) => {
if (isAtTop) loadMore?.(messageLimit);
if (isAtTop) {
loadMore?.(messageLimit);
}
};

useEffect(() => {
Expand Down
12 changes: 12 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2497,6 +2497,13 @@
dependencies:
"@types/lodash" "*"

"@types/lodash.defaultsdeep@^4.6.9":
version "4.6.9"
resolved "https://registry.yarnpkg.com/@types/lodash.defaultsdeep/-/lodash.defaultsdeep-4.6.9.tgz#050fbe389a7a6e245b15da9ee83a8a62f047a1c4"
integrity sha512-pLtCFK0YkHfGtGLYLNMTbFB5/G5+RsmQCIbbHH8GOAXjv+gDkVilY98kILfe8JH2Kev0OCReYxp1AjxEjP8ixA==
dependencies:
"@types/lodash" "*"

"@types/lodash.throttle@^4.1.7":
version "4.1.7"
resolved "https://registry.yarnpkg.com/@types/lodash.throttle/-/lodash.throttle-4.1.7.tgz#4ef379eb4f778068022310ef166625f420b6ba58"
Expand Down Expand Up @@ -9391,6 +9398,11 @@ lodash.deburr@^4.1.0:
resolved "https://registry.yarnpkg.com/lodash.deburr/-/lodash.deburr-4.1.0.tgz#ddb1bbb3ef07458c0177ba07de14422cb033ff9b"
integrity sha1-3bG7s+8HRYwBd7oH3hRCLLAz/5s=

lodash.defaultsdeep@^4.6.1:
version "4.6.1"
resolved "https://registry.yarnpkg.com/lodash.defaultsdeep/-/lodash.defaultsdeep-4.6.1.tgz#512e9bd721d272d94e3d3a63653fa17516741ca6"
integrity sha512-3j8wdDzYuWO3lM3Reg03MuQR957t287Rpcxp1njpEa8oDrikb+FwGdW3n+FELh/A6qib6yPit0j/pv9G/yeAqA==

lodash.escaperegexp@^4.1.2:
version "4.1.2"
resolved "https://registry.yarnpkg.com/lodash.escaperegexp/-/lodash.escaperegexp-4.1.2.tgz#64762c48618082518ac3df4ccf5d5886dae20347"
Expand Down

0 comments on commit 3685030

Please sign in to comment.