Fix / TVOD infinite loop and render optimizations

[ChristiaanScheermeijer]

Description

This PR fixes an infinite render loop when opening the Choose Offer modal for a JWP TVOD platform. This was caused by the clientOffers being constructed each render while being a dependency in the useOffers hook.

Besides that, for a TVOD platform, the clientOffer resulted in ['null'] when not set. This causes the useOffers hook to request non-existing offers for JWP and Cleeng.

The other commit fixes some unnecessary renders in the Choose Offer form caused by depending on the location or navigate (which also depends on location) reactive variables.

[github-actions]

Visit the preview URL for this PR (updated for commit 309791d):

https://ottwebapp--pr288-fix-tvod-fix-infinit-qgcetuia.web.app

_{(expires Sat, 24 Jun 2023 15:06:39 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: c198f8a3a199ba8747819f7f1e45cf602b777529}

[dbudzins]

Is there a reason we can't just use string literals here?

Suggested change

	notifyOnChangeProps,
	['data', 'isLoading'],

[ChristiaanScheermeijer]

Without the as const modifiers, the type results in string[] and is not accepted by react-query.

If I use ['data', 'isLoading'] as const the type results in readonly ['data', 'isLoading'] and is also not accepted by react-query.

This does work but is not much cleaner IMHO

const notifyOnChangeProps: QueryObserverOptions['notifyOnChangeProps'] = ['data', 'isLoading'];

[dbudzins]

Should we try to support monthly OR yearly as well?

[dbudzins]

Do you need dependencies on navigate and location here and below?

[ChristiaanScheermeijer]

It should be in a regular useCallback, but not in the useEventCallback.

We've found that when a modal has an effect, memo or callback on the location/navigate properties, all will trigger an update when closing the modal because of the location update.

Because this is hard to discover, we've chosen to wrap these dependant functions with useEventCallback to ensure functions depending will not update unnecessarily (and possibly cause issues).

For example, this useEffect:

useEffect(() => {
    // close auth modal when there are no offers defined in the config
    if (!isLoading && !offers.length) {
      closeModal(true);
    }
  }, [isLoading, offers, closeModal]);

This effect closes the modal (only once) based on the reactive variables isLoading and offers. The closeModal is still considered a reactive variable, but never updates because wrapped in useEventCallback.

The problem without the useEventCallback is that we can't ensure that this effect is only running when isLoading or offers are changed. Because it will also run when location changes (after a navigate).

[dbudzins]

Same question if we need dependencies on navigate and location?