frontloadMeta.pending when nothing is returned

[matthewlein]

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

react-frontload/src/index.tsx

Lines 408 to 429 in a206364

	const frontloadState = React.useContext(FrontloadContext)!
	const serverRendered = frontloadState.isFirstRender()
	const pendingInitial = serverRendered
	? !frontloadState.hasFrontloadRunOnServer(key)
	: true
	const data = serverRendered
	? frontloadState.getFrontloadServerRenderedData(key)
	: undefined
	const error = !!data?._____FRONTLOAD_____isServerRenderError
	const [state, setState] = React.useState<{
	data: T
	frontloadMeta: FrontloadMeta
	}>({
	data,
	frontloadMeta: {
	serverRendered,
	pending: pendingInitial,
	done: !pendingInitial,
	error,
	},
	})

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

   react-frontload/src/index.tsx

   Line 196 in a206364

   this.serverRender.cache[key] = data

   )

What do you think?

[davnicwil]

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 :-)

[matthewlein]

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

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