react-frontload icon indicating copy to clipboard operation
react-frontload copied to clipboard

frontloadMeta.pending when nothing is returned

Open matthewlein opened this issue 3 years ago • 2 comments

Upgrading to v2 and curious about how frontloadMeta.pending works. I have noticed that when nothing is returned from the useFrontload function, there is nothing in the frontloadServerRender.data. And if I'm reading this right https://github.com/davnicwil/react-frontload/blob/a2063644e23dfee559fee47dd88575872a773e7b/src/index.tsx#L408-L429 it uses the keys as an indicator for whether frontload has run or not.

Here's an example of a redux thunk that doesn't need to return anything, because it is handled in the redux store.

useFrontload('ArticleListContainer', async () => {
  await dispatch(fetchAllArticles());
});

So if nothing is returned, it seems like frontloadMeta.pending is always true.

By adding a simple return true

useFrontload('ArticleListContainer', async () => {
  await dispatch(fetchAllArticles());
  return true;
});

frontloadMeta.pending works as expected.

It seems like the options are to either:

  1. Require/recommend that people return something from every useFrontload function, update the docs to include that.
  2. Return something like true in the absence of a returned value (maybe here https://github.com/davnicwil/react-frontload/blob/a2063644e23dfee559fee47dd88575872a773e7b/src/index.tsx#L196)

What do you think?

matthewlein avatar Jan 19 '22 22:01 matthewlein

Great question!

So in V2 this is something that falls out of the design of making data & state first-class within Frontload - that is, unlike V1, the assumption is that a useFrontload always returns some data which is then directly used by the component.

Unlike V1 there's no assumption that any other 'global' data store like redux is being used. It can be, of course, but this should be considered strictly a side-effect in the V2 model - the assumption remains that the useFrontload should still return something - like a pure data loading function (the FP terms here are, of course, just an analogy :-)

That's why pending depends on something being returned. In fact under the V2 model a useFrontload not returning anything probably represents a bug. I think returning an explicit 'true' or some other marker value for useFrontloads that purely run side effects, and keeping this 'returning undefined is a de facto bug' behaviour, is overall a good thing.

For this reason, I think I'd prefer to deliberately not do (2) and keep the model very explicit.

I would say though that a dev-mode warning about this could be useful, and also doing (1) and making this very clear in the docs is a great idea.

If you'd be up for doing either/both of these I will happily accept a PR :-)

davnicwil avatar Jan 23 '22 18:01 davnicwil

I really like the idea of a dev-mode warning.

Similarly, a warning for duplicate keys detected, like react does, would be useful.

matthewlein avatar Jan 24 '22 16:01 matthewlein