Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pageviews worden dubbel gemeten #38

Open
pepijnvandenhoven opened this issue Oct 18, 2024 · 7 comments
Open

Pageviews worden dubbel gemeten #38

pepijnvandenhoven opened this issue Oct 18, 2024 · 7 comments

Comments

@pepijnvandenhoven
Copy link

pepijnvandenhoven commented Oct 18, 2024

Voorbeeld in https://github.com/freshheads/analytics-essentials/blob/develop/doc/mixpanel_setup.md#page-view-events updaten.

  • pathname geeft TS error: Type 'null' is not assignable to type 'string | undefined'.ts(2322)
  • exhaustive-deps warning: React Hook useEffect has missing dependencies: 'router.route' and 'trackPageView'. Either include them or remove the dependency array.

Wanneer je alle deps include, wordt de initiële homepage pageview de dubbel gemeten (ook in prod). Dit komt doordat router.isReady dan false is.

image

Voorstel:

export const TrackPageView = () => {
  const { trackPageView } = useMixpanelContext();
  const { route, isReady } = useRouter();
  const pathname = usePathname();

  useEffect(() => {
    if (!isReady) {
      return;
    }
    trackPageView({
      data: {
        title: document.title,
        pathname: pathname ?? '/',
        route: route,
      },
    });
  }, [trackPageView, route, pathname, isReady]);

  return null;
};
@gitknul
Copy link
Member

gitknul commented Oct 18, 2024

Ja, dit lijkt me een prima implementatie.

@martinbroos
Copy link
Member

martinbroos commented Oct 28, 2024

Komt het dubbel meten niet doordat je dit in dev mode checkt waar componenten 2 keer gerendered worden?
Omdat er nu alleen een pathname staat zou je eigenlijk niet zeggen dat hij nogmaals geladen worden maar dat in dat geval de route niet up to date is. Maar dat is natuurlijk ook een probleem en dat los je dan weer wel op door die route als dependency mee te geven. En dan wordt die isReady waarschijnlijk wel noodzakelijk.

Kortom, denk wel dat het een goede aanvulling is. Misschien zouden we er ook bij moeten zetten dat dit voorbeeld op basis van Next.JS is gemaakt.

We hebben al wel een aantal implementaties op de originele manier gedaan, zou wel goed zijn dan om die eens na te lopen. Die zouden nu dus dubbel gemeten worden.

@pepijnvandenhoven
Copy link
Author

pepijnvandenhoven commented Oct 31, 2024

@martinbroos ik heb het ff dubbel gecheckt. Met de huidige implementatie zijn twee dingen mis, nl. TS error en exhaustive-deps warning. Als je deze twee dingen oplost, wordt de homepage initiëel dubbel gemeten, ook in de prod build (trouwens, in dev mode worden page views ook maar 1 keer gemeten, dus dat is goed). Ik heb de issue description hier aangepast.

Als je alleen pathname als dependency op je useEffect hebt, wordt het niet dubbel gemeten.

@pepijnvandenhoven
Copy link
Author

pepijnvandenhoven commented Oct 31, 2024

Nu we het hier toch over hebben... wat is het verschil tussen pathname: pathname en route: route? Deze lijken dezelfde waarde te bevatten, maar die laatste is niet gedocumenteerd en lijkt outdated

{
    "router.pathname": "/vacatures/[id]", // deze wordt niet gebruikt, maar even ter illustratie
    "router.route": "/vacatures/[id]",
    "router.asPath": "/vacatures/409", // deze wordt niet gebruikt, maar even ter illustratie
    "pathname (usePathname)": "/vacatures/409"
}

@KevinvdBurg
Copy link
Member

Hi, ik had toevallig hetzelfde probleem waarbij pagina's dubbel werden getrackt. Ik ontdekte dat dit komt door de StrictMode in React.

StrictMode aan:
image

StrictMode uit:
image

Het gebruik van StrictMode is overigens wel nuttig in de ontwikkelomgeving, omdat het helpt om bugs sneller te traceren: React StrictMode.

De suggestie van @pepijnvandenhoven om isReady te gebruiken gaf bij mij nog steeds een dubbele render.

Daarom heb ik een aangepaste versie gemaakt van TrackPageView die in StrictMode slechts één keer de TrackEvent triggert:

import { useEffect, useRef } from 'react';
import { useRouter } from 'next/router';
import { useMixpanelContext } from '@freshheads/analytics-essentials';

export default function TrackPageView(): null {
    const { pathname, isReady } = useRouter();
    const { trackPageView, setEventContext } = useMixpanelContext();

    const lastTrackedPageRef = useRef<string | null>(null);
    const lastEventContextRef = useRef<string | null>(null);

    useEffect(() => {
        if (!isReady) return;

        const currentPage = {
            pathname: pathname,
            href: window.location.href,
            title: document.title,
        };

        const currentPageString = JSON.stringify(currentPage);

        if (
            process.env.NODE_ENV === 'development' &&
            lastTrackedPageRef.current === currentPageString
        ) {
            return;
        }

        lastTrackedPageRef.current = currentPageString;

        trackPageView({
            data: currentPage,
        });
    }, [pathname, isReady, trackPageView]);

    useEffect(() => {
        const newEventContext = {
            href: window.location.href || window.location.pathname,
        };

        const newEventContextString = JSON.stringify(newEventContext);

        if (
            process.env.NODE_ENV === 'development' &&
            lastEventContextRef.current === newEventContextString
        ) {
            return;
        }

        lastEventContextRef.current = newEventContextString;

        setEventContext(newEventContext);
    }, [pathname, setEventContext]);

    return null;
}

@martinbroos
Copy link
Member

Nu we het hier toch over hebben... wat is het verschil tussen pathname: pathname en route: route? Deze lijken dezelfde waarde te bevatten, maar die laatste is niet gedocumenteerd en lijkt outdated

{
    "router.pathname": "/vacatures/[id]", // deze wordt niet gebruikt, maar even ter illustratie
    "router.route": "/vacatures/[id]",
    "router.asPath": "/vacatures/409", // deze wordt niet gebruikt, maar even ter illustratie
    "pathname (usePathname)": "/vacatures/409"
}

@pepijnvandenhoven Het idee van een route meegeven is het onderscheid kunenn maken tussen job/1234 en de route /job/:id zodat je eventueel die detail pagina's allemaal op 1 hoop kan gooien in mixpanel. Maar omdat dit per implementatie (nextJs of react-router-dom) anders kan werken zit het niet in de docs. Maar als je er bij kan is het denk ik wel een nuttige waarde. De type maakt het volgens mij ook niet verplicht om mee te geven.

@martinbroos
Copy link
Member

martinbroos commented Nov 1, 2024

@KevinvdBurg ik zou voor de strict mode (dev mode) geen uitzonderingen maken. Je test een keer of tracking werkt en vervolgens zet je het toch uit lijkt me. Strict mode zal in een productie build componenten niet 2 keer gaan renderen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants