Skip to content

Commit

Permalink
FIX: Feed, FlatFeed, NotificationFeed - options change wouldn't tri…
Browse files Browse the repository at this point in the history
…gger refresh (#313)

* Remove refresh from NotificationFeed and FlatFeed

* Added notify and options to dependency arrays

* Fixed subscribe and unsubscribe methods

* Removed unnecessary brackets

* Remove client from the condition

Co-authored-by: Amin Mahboubi <amin@getstream.io>

* Added comment

* Added return statement for better redability

* Added custom useDeepCompareEffect

* Added setter for options

* Added minor updates

* Updated options setting

Co-authored-by: Amin Mahboubi <amin@getstream.io>
  • Loading branch information
arnautov-anton and Amin Mahboubi authored Nov 3, 2021
1 parent 2384bfb commit b67a2b7
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 44 deletions.
6 changes: 1 addition & 5 deletions src/components/FlatFeed.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect } from 'react';
import React from 'react';
import { EnrichedActivity, UR } from 'getstream';
import { LoadingIndicator as DefaultLoadingIndicator, LoadingIndicatorProps } from 'react-file-utils';

Expand Down Expand Up @@ -96,10 +96,6 @@ const FlatFeedInner = <

const refreshFeed = () => feed.refresh(options);

useEffect(() => {
refreshFeed();
}, [feed.feedGroup, feed.userId]);

if (feed.refreshing && !feed.hasDoneRequest) {
return <div className="raf-loading-indicator">{smartRender<LoadingIndicatorProps>(LoadingIndicator)}</div>;
}
Expand Down
2 changes: 0 additions & 2 deletions src/components/NotificationFeed.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,6 @@ const NotificationFeedInner = <
const refreshFeed = () => feed.refresh(options);

useEffect(() => {
refreshFeed();

return () => {
feed.activities.clear();
feed.activityOrder.splice(0, feed.activityOrder.length);
Expand Down
37 changes: 25 additions & 12 deletions src/context/Feed.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { PropsWithChildren, useContext, useEffect, useMemo, useState } from 'react';
import React, { PropsWithChildren, useContext, useEffect, useMemo, useState, useRef } from 'react';
import {
Activity,
GetFeedOptions,
Expand All @@ -15,6 +15,7 @@ import {

import { FeedManager } from './FeedManager';
import { DefaultAT, DefaultUT, useStreamContext } from './StreamApp';
import isEqual from 'lodash/isEqual';

export type FeedContextValue<
UT extends DefaultUT = DefaultUT,
Expand Down Expand Up @@ -148,29 +149,41 @@ export function Feed<
CRT,
PT
>();
const { feedGroup, userId, children } = props;
const { feedGroup, userId, children, options, notify } = props;
const [, setForceUpdateState] = useState(0);

const optionsReference = useRef<GetFeedOptions | undefined>();

if (!isEqual(optionsReference.current, options)) optionsReference.current = options;

const feedId = client?.feed(feedGroup, userId).id;

const manager = useMemo(() => {
if (!client) return null;
// const clientDifferent = this.props.client !== prevProps.client;
// const notifyDifferent = this.props.notify !== prevProps.notify;
// const feedDifferent = this.props.userId !== prevProps.userId || this.props.feedGroup !== prevProps.feedGroup;
// const optionsDifferent = !_isEqual(this.props.options, prevProps.options);
// if (clientDifferent || feedDifferent || optionsDifferent || notifyDifferent)
// TODO: Implement
const feedId = client.feed(feedGroup, userId).id;
if (!feedId) return null;

// TODO: check if any of the clients changed
return (
sharedFeedManagers[feedId] ||
new FeedManager<UT, AT, CT, RT, CRT, PT>({ ...props, analyticsClient, client, user, errorHandler })
);
}, [feedGroup, userId]);
}, [feedId]);

useEffect(() => {
const forceUpdate = () => setForceUpdateState((prevState) => prevState + 1);
if (manager) manager.props.notify = notify;
manager?.register(forceUpdate);
return () => manager?.unregister(forceUpdate);
}, [manager]);
}, [manager, notify]);

useEffect(() => {
if (!manager) return;

if (optionsReference.current) {
manager.props.options = optionsReference.current;
}

manager.refresh();
}, [manager, optionsReference.current]);

if (!manager) return null;

Expand Down
52 changes: 28 additions & 24 deletions src/context/FeedManager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -992,35 +992,35 @@ export class FeedManager<
return this.setState(newState);
};

// TODO: deprecate async in next major release
// eslint-disable-next-line require-await
subscribe = async () => {
if (this.props.notify) {
const feed = this.feed();
await this.setState((prevState) => {
if (prevState.subscription) {
return {};
}
if (!this.props.notify) return;

const subscription = feed.subscribe((data) => {
this.setState((prevState) => {
const numActivityDiff = data.new.length - data.deleted.length;

return {
realtimeAdds: prevState.realtimeAdds.concat(data.new),
realtimeDeletes: prevState.realtimeDeletes.concat(data.deleted),
unread: prevState.unread + numActivityDiff,
unseen: prevState.unseen + numActivityDiff,
};
});
const feed = this.feed();
this.setState((prevState) => {
if (prevState.subscription) return {};

const subscription = feed.subscribe((data) => {
this.setState((prevState) => {
const numActivityDiff = data.new.length - data.deleted.length;

return {
realtimeAdds: prevState.realtimeAdds.concat(data.new),
realtimeDeletes: prevState.realtimeDeletes.concat(data.deleted),
unread: prevState.unread + numActivityDiff,
unseen: prevState.unseen + numActivityDiff,
};
});
});

subscription.then(
() => console.log(`now listening to changes in realtime for ${this.feed().id}`),
(err) => console.error(err),
);
subscription.then(
() => console.log(`now listening to changes in realtime for ${this.feed().id}`),
(err) => console.error(err),
);

return { subscription };
});
}
return { subscription };
});
};

unsubscribe = async () => {
Expand All @@ -1031,8 +1031,12 @@ export class FeedManager<

try {
await subscription;

this.setState({ subscription: null });

// @ts-expect-error
subscription?.cancel();

console.log(`stopped listening to changes in realtime for ${this.feed().id}`);
} catch (err) {
console.error(err);
Expand Down
1 change: 0 additions & 1 deletion src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import relativeTime from 'dayjs/plugin/relativeTime';
import { EnrichedUser, UR } from 'getstream';
import { TDateTimeParser } from '../i18n/Streami18n';
import { DefaultUT } from '../context/StreamApp';

Dayjs.extend(utc);
Dayjs.extend(minMax);
Dayjs.extend(relativeTime);
Expand Down

0 comments on commit b67a2b7

Please sign in to comment.