fix: prevented 'no message' screen on first narrow load

[anshuman-8]

Fixed #5152

  const nothingKnown = messages.length === 0 && !caughtUp.older && !caughtUp.newer;
  useConditionalEffect(scheduleFetch, nothingKnown);

  const fetching = useSelector(state => getFetchingForNarrow(state, narrow));
  const isFetching = fetching.older || fetching.newer || loading || nothingKnown;

Fixed the issue by adding nothingKnown (if narrow data is not loaded) as a factor in isFetching to decide if to show the loading placeholder, preventing “No Message” screen.

[chrisbobbe]

Thanks, @anshuman-8!

I think this new logic will make the app behave closer to correctly, but I wonder if isFetching is really the right name anymore, since for a frame or two it could be true just because nothingKnown is true, and during that time we won't actually be fetching messages.

(It's already kind of a confusing name, because it might be true just because loading is true, and during that time we'll be doing the "initial fetch" (see registerAndStartPolling) but not a message fetch.)

I haven't yet thought of a better name; perhaps @gnprice has some ideas on that point and on the following things I noticed while looking at the surrounding code 🙂:

• Do we really want loading, which is the message-fetch progress indicator, to factor into isFetching? During /register, if there's an active, logged-in account, I think we should show stale data even if that stale data is "there are no messages". We'll already mark the loading state more globally, e.g., as we currently do with the "Connecting…" bar.

• Should we change nothingKnown like this:

  - messages.length === 0 && !caughtUp.older && !caughtUp.newer
  + messages.length === 0 && !(caughtUp.older && caughtUp.newer)

  Theoretically that would catch more cases where "we don't know any [messages], and nor do we know if there are some", to quote nothingKnown's code comment.

• Is the selector isFetchNeededAtAnchor something we want to keep around, and if so, does the code around here (perhaps nothingKnown) want to use it?

• If we're using caughtUp to choose between showMessagePlaceholders and sayNoMessages, then is there any point in checking fetching too? Maybe we can remove fetching from the expressions to simplify things.

(Greg please don't feel like you have to settle all those questions before we make progress on #5152.)

[chrisbobbe]

Ah, and another thing I noticed: when we resolve this TODO on nothingKnown, we should remove it so future readers don't think there's still something to do 🙂:

  // TODO: We may still briefly show NoMessages; this render will have
  //   isFetching false, even though the fetch effect will cause a rerender
  //   with isFetching true.  It'd be nice to avoid that.