Skip to content

Commit

Permalink
ChatScreen [nfc]: Replace useEdgeTriggeredEffect with useConditionalE…
Browse files Browse the repository at this point in the history
…ffect

And remove useConditionalEffect; see the previous commit for why
useConditionalEffect has a better interface.
  • Loading branch information
chrisbobbe committed Mar 29, 2022
1 parent 39be99c commit e999399
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 37 deletions.
10 changes: 5 additions & 5 deletions src/chat/ChatScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { getAuth, getCaughtUpForNarrow } from '../selectors';
import { showErrorAlert } from '../utils/info';
import { TranslationContext } from '../boot/TranslationProvider';
import * as api from '../api';
import { useEdgeTriggeredEffect } from '../reactUtils';
import { useConditionalEffect } from '../reactUtils';

type Props = $ReadOnly<{|
navigation: AppNavigationProp<'chat'>,
Expand Down Expand Up @@ -71,9 +71,9 @@ const useMessagesWithFetch = args => {
// like using instance variables in class components:
// https://reactjs.org/docs/hooks-faq.html#is-there-something-like-instance-variables
const shouldFetchWhenNextFocused = React.useRef<boolean>(false);
const scheduleFetch = () => {
const scheduleFetch = useCallback(() => {
shouldFetchWhenNextFocused.current = true;
};
}, []);

const [fetchError, setFetchError] = React.useState<Error | null>(null);

Expand All @@ -91,7 +91,7 @@ const useMessagesWithFetch = args => {
if (eventQueueId !== null) {
scheduleFetch();
}
}, [eventQueueId]);
}, [eventQueueId, scheduleFetch]);

// If we stop having any data at all about the messages in this narrow --
// we don't know any, and nor do we know if there are some -- then
Expand All @@ -103,7 +103,7 @@ const useMessagesWithFetch = args => {
// isFetching false, even though the fetch effect will cause a rerender
// with isFetching true. It'd be nice to avoid that.
const nothingKnown = messages.length === 0 && !caughtUp.older && !caughtUp.newer;
useEdgeTriggeredEffect(scheduleFetch, nothingKnown, true);
useConditionalEffect(scheduleFetch, nothingKnown);

// On first mount, fetch. (This also makes a fetch no longer scheduled,
// so the if-scheduled fetch below doesn't also fire.)
Expand Down
32 changes: 0 additions & 32 deletions src/reactUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,38 +84,6 @@ export const useHasStayedTrueForMs = (value: boolean, duration: number): boolean
return result;
};

/**
* Invoke the callback as an effect when `value` turns from false to true.
*
* This differs from putting `value` in a `useEffect` dependency list in
* that the callback will *not* be invoked when the value changes in the
* other direction, from true to false.
*
* `includeStart` controls what happens if `value` is true on the very first
* render. If `includeStart` is true, then the effect is triggered (so
* treating its previous state of nonexistence as equivalent to being
* false). If `includeStart` is false, then the effect is not triggered (so
* treating its previous state of nonexistence as not a valid state to
* compare to.)
*
* The callback is not permitted to return a cleanup function, because it's
* not clear what the semantics should be of when such a cleanup function
* would be run.
*/
export const useEdgeTriggeredEffect = (
cb: () => void,
value: boolean,
includeStart: boolean,
): void => {
const prev = usePrevious(value, !includeStart);
useEffect(() => {
if (value && !prev) {
cb();
}
// No dependencies list -- the effect itself decides whether to act.
});
};

/**
* Like `useEffect`, but the callback only runs when `value` is true.
*
Expand Down

0 comments on commit e999399

Please sign in to comment.