Shopify / hydrogen

Hydrogen lets you build faster headless storefronts in less time, on Shopify.
https://hydrogen.shop
MIT License
1.41k stars 267 forks source link

404 (Not Found) pages break Analytics #2576

Closed dcodrin closed 2 weeks ago

dcodrin commented 3 weeks ago

What is the location of your example repository?

https://github.com/Shopify/hydrogen/tree/main/templates/skeleton

Which package or tool is having this issue?

Hydrogen

What version of that package or tool are you using?

skeleton@2024.7.8

What version of Remix are you using?

^2.10.1

Steps to Reproduce

Reproducible with skeleton template.

  1. DO NOT use the mock shop as it requires the customer privacy api.
  2. Navigate to a page that will throw a 404 e.g. /does-not-exist
  3. Click on the Mock.shop logo to client side navigate to the homepage
  4. Observe the error below in the console. The only way to get analytics to work again is with a full page refresh.
Make sure to load the Shopify Customer Privacy API with useCustomerPrivacy() or <AnalyticsProvider>. function pageViewHandler(payload) {
  const eventPayload = prepareBasePageViewPayload(payload);
  if (!eventPayload) return;
  sendShopifyAnalytics({
    eventName: AnalyticsEventName.PAGE_VIEW_2,
    payload: {
      ...eventPayload,
      ...viewPayload
    }
  });
  viewPayload = {};
} Error: Shopify Customer Privacy API not available. Must be used within a useEffect. Make sure to load the Shopify Customer Privacy API with useCustomerPrivacy() or <AnalyticsProvider>.

Image

Expected Behavior

Analytics should work after encountering a 404.

Actual Behavior

Analytics STOPS working after encountering a 404.

scottdixon commented 3 weeks ago

Thanks for flagging this @dcodrin. I'm able to replicate.

@wizardlyhel - seems like window.privacyBanner becomes undefined.

dcodrin commented 3 weeks ago

It seems like the issue is caused by the Layout being unmounted when an error is thrown. I expected this to no longer be the case since we’re using the Layout Export as per the Remix documentation.

I managed to work around this by lifting the ErrorBoundary to ($locale).tsx. This resolved the issue however, I’m not entirely sure if there are any downsides to doing this?

import {isRouteErrorResponse, useRouteError} from '@remix-run/react';
import type {LoaderFunctionArgs} from '@shopify/remix-oxygen';

import {GenericError} from '~/components/GenericError';
import {NotFound} from '~/components/NotFound';

export async function loader({params, context}: LoaderFunctionArgs) {
  const {language, country} = context.storefront.i18n;

  if (
    params.lang &&
    params.lang.toLowerCase() !== `${language}-${country}`.toLowerCase()
  ) {
    // If the locale URL param is defined, yet we are still at the default locale
    // then the locale param must be invalid, send to the 404 page
    throw new Response(null, {status: 404});
  }

  return null;
}

export function ErrorBoundary() {
  const routeError = useRouteError();
  const isRouteError = isRouteErrorResponse(routeError);

  return isRouteError ? (
    <>
      {routeError.status === 404 ? (
        <NotFound />
      ) : (
        <GenericError
          error={{message: `${routeError.status} ${routeError.data}`}}
        />
      )}
    </>
  ) : (
    <GenericError error={routeError as Error} />
  );
}

According to the Remix docs the Layout Export is supposed to avoid mounting and re-mounting the Layout when an error is thrown but again this does not seem to be working as intended or perhaps i'm missing something here:

Avoids React from re-mounting your app shell elements when switching between the root component/HydrateFallback/ErrorBoundary which can cause a FOUC if React removes and re-adds tags from your component.

I have been having issues with external scripts as well because of this behaviour, and again the only workaround i found is to lift the ErrorBoundary.

Cheers!

wizardlyhel commented 2 weeks ago

@dcodrin I have a fix coming for this. https://github.com/Shopify/hydrogen/pull/2590

ErrorBoundary are working as they should. This is a problem within the Analytics.Provider. I'll have this ship out next week before we make a major release cut for 2024-10.