Removing this check might result in the wrong template being returned on block themes for a split second before the home page actually loads, so maybe there's a way to keep this check for block themes somehow?

Yes, if the homepage is not yet resolved, but exists, the selector may briefly return a wrong result. Instead of returning the front-page template, it will returns something else, like the default template for post pages etc.

Interesting that there are two other usages of getHomeTemplate where there is the same issue: useDefaultTemplateLabel and getDefaultTemplateLabel. All three functions have a copy&paste of the same logic, but only one returns early when homepage is null.

I think what we really want is that getHomePage has different return values for "still resolving" and "there is no homepage". In getHomePage we can achieve that by checking the resolution status of getDefaultTemplateId:

const query = { slug: 'front-page' };
const frontPageTemplateId = select( STORE_NAME ).getDefaultTemplateId( query );
if ( frontPageTemplateId ) {
  return { postType: 'wp_template', postId: frontPageTemplateId };
} else if ( select( STORE_NAME ).hasFinishedResolution( 'getDefaultTemplateId', [ query ] ) ) {
  return {}; // resolution finished and yet no template: empty homepage
} else {
  return null; // still resolving
}

After this, callers of getHomePage can propertly branch on whether the home page is still loading, or is definitely empty.

Yeah, I thought it might fine to have this brief change, as I describe it in the PR, but it might be better to follow the quite common pattern for selectors to return null while resolving.

I'm still not sure about my updates. I'll resolve this discussion since I removed the code and comment on the new code.

Yes, changing the return value of getDefaultTemplateId to '' vs null also works. We're slightly changing behavior of a public selector, I'm not sure how serious is it from back-compat perspective.

It would be also nice to modify useDefaultTemplateLabel and getDefaultTemplateLabel so that the behavior of all three functions is exactly the same. It's a pity that the fields package duplicates a complex logic from core-data -- maybe we need to create or reuse a selector that contains that logic just once?

I'm not sure how serious is it from back-compat perspective.

I'm not sure about that.. It's still a falsy value, but consumers could check for undefined explicitly? 🤔

For now I'll remove the template field utils (getDefaultTemplateLabel) since it seems they are not used.

This might be related to the comments below about cache invalidation.

I couldn't use hasFinishedResolution (always false) because probably it's affected by createSelector and the lack of access to state.metadata (where hasFinishedResolution is included).

That's the reason for checking for empty string and also setting an empty string to getDefaultTemplateId when the request has resolved (see).

Lot's of interconnected complex pieces and I'm not 100% confident about the fix.

You mean, you can't call hasFinishedResolution for getDefaultTemplateId, it returns bad results? That's strange. It would likely be a bug in the select function we're passing to selectors, maybe the privateness of the selector is also relevant.

We certainly can't call hasFinishedResolution for getTemplateId, because it's a selector that doesn't have a resolver. It only combines data from other resolver-enabled sub-selectors. Here it's expected that the return value is always false.

hasFinishedResolution for getDefaultTemplateId seems to be always false. If we don't use createSelector it works just fine.

I debugged the hasFinishedResolution mystery. What happens is:

• after the getDefaultTemplateId resolver sets the new ID in the state, all store listeners run synchronously, before the dispatch call returns. The getHomePage selector runs again because it's triggered by the listener.
• but the finishResolution action is dispatched only a little later. Now it triggers the getHomePage selector once more, but because the resolution state is not part of the createSelector deps, and probably can't even be, the value is not recomputed. The old cached return value is returned again.

So, hasFinishedResolution is not always false, it becomes true, but at that time it's too late.

There is not even any timeout between the two dispatches, the code is similar to:

dispatch( receiveDefaultTemplateId() );
dispatch( finishResolution() );

It's all synchronous and very close to each other. But still late.

Can we keep all the dispatches in one batch? If we don't there is this code in getTemplateId:

const homepage = select( STORE_NAME ).getHomePage();
const templates = select( STORE_NAME ).getEntityRecords( 'postType', 'wp_template' );

The receiveDefaultTemplateId, dispatch sets a valid homepage, but then the getEntityRecords selector will be called before the receiveEntityRecords dispatch ran. That could cause an inefficiency. One extra selector call at best, an extra REST request at worst.