PostHog / posthog-js

Send usage data from your web app or site to PostHog, with autocapture.
https://posthog.com/docs/libraries/js
Other
280 stars 118 forks source link

Next.js error pre-rendering pages since 1.112.0 #1139

Open danilofuchs opened 5 months ago

danilofuchs commented 5 months ago

Bug description

Using Next.js Pages Router, PostHog/React crashes build for static pages

posthog-js >= 1.112.0

(I tested all 1.111.x builds and they work as expected, anything from 1.112.0 and above is broken for us)

How to reproduce

  1. Create Next.js 14.1.4 project with pages router, Typescript, Tailwind
  2. Add PostHog provider to _app.tsx:
    
    // PostHogProvider.tsx
    import { useUser } from "@clerk/nextjs";
    import { PostHogProvider as Provider, usePostHog } from "posthog-js/react";
    import { useEffect } from "react";
    import { env } from "~/env.js";

interface Props { children: React.ReactNode; }

export const PostHogProvider = ({ children }: Props) => { return (

{children}

); };

const PostHogUserSync = () => { const postHog = usePostHog(); const { user, isLoaded, isSignedIn } = useUser();

useEffect(() => { if (user && isLoaded && isSignedIn) { postHog.identify(user.id, { email: user.primaryEmailAddress?.emailAddress, companyId: user.publicMetadata.companyId, role: user.publicMetadata.role, }); }

if (isLoaded && !isSignedIn) {
  postHog.reset();
}

}, [user, isLoaded, isSignedIn, postHog]);

return null; };

```typescript
// _app.tsx
return (
  <PostHogProvider>
    <Component {...pageProps} />
  </PostHogProvider>
)
  1. Add a static pages
  2. next build
Error occurred prerendering page "/page". Read more: https://nextjs.org/docs/messages/prerender-error

TypeError: L is not a constructor
    at ir (/home/danilo/salvy/salvy-dashboard/node_modules/posthog-js/dist/module.js:1:77114)
    at /home/danilo/salvy/salvy-dashboard/node_modules/posthog-js/dist/module.js:1:108975
    at e.value (/home/danilo/salvy/salvy-dashboard/node_modules/posthog-js/dist/module.js:1:108987)
    at e.value (/home/danilo/salvy/salvy-dashboard/node_modules/posthog-js/dist/module.js:1:62848)
    at e.value (/home/danilo/salvy/salvy-dashboard/node_modules/posthog-js/dist/module.js:1:107558)
    at e.value (/home/danilo/salvy/salvy-dashboard/node_modules/posthog-js/dist/module.js:1:106745)
    at e.value (/home/danilo/salvy/salvy-dashboard/node_modules/posthog-js/dist/module.js:1:103481)
    at /home/danilo/salvy/salvy-dashboard/node_modules/posthog-js/react/dist/umd/index.js:30:47
    at Object.Kc [as useMemo] (/home/danilo/salvy/salvy-dashboard/node_modules/react-dom/cjs/react-dom-server.browser.production.min.js:60:240)
    at exports.useMemo (/home/danilo/salvy/salvy-dashboard/node_modules/react/cjs/react.production.min.js:25:208)
[PostHog.js] was already loaded elsewhere. This may cause issues.
[PostHog.js] was already loaded elsewhere. This may cause issues.
[PostHog.js] was already loaded elsewhere. This may cause issues.
[PostHog.js] was already loaded elsewhere. This may cause issues.
[PostHog.js] was already loaded elsewhere. This may cause issues.
[PostHog.js] was already loaded elsewhere. This may cause issues.
[PostHog.js] was already loaded elsewhere. This may cause issues.
[PostHog.js] was already loaded elsewhere. This may cause issues.
[PostHog.js] was already loaded elsewhere. This may cause issues.
[PostHog.js] was already loaded elsewhere. This may cause issues.
[PostHog.js] was already loaded elsewhere. This may cause issues.

Additional context

L is minified, appears to be an HTTP fetcher image, probably related to https://github.com/PostHog/posthog-js/pull/1055 from 1.112.0

Thank you for your bug report – we love squashing them!

benjackwhite commented 5 months ago

I checked this out just now with the example app in the repository and no issues. Does this occur on the latest version?

danilofuchs commented 5 months ago

I checked this out just now with the example app in the repository and no issues. Does this occur on the latest version?

It does occur on the latest version. I'm having a bad time debugging this because of the minified code. I will investigate further next week

lightster commented 5 months ago

Hi @benjackwhite and team,

My team ran into a similar problem this week trying to upgrade from a very old posthog-js version (1.67) and we dug pretty deep into the problem.

The reason you are not able to reproduce the issue using the Next.js example app in this repository is because the example app initializes the PostHog client on its own and then passes that client to <PostHogProvider>. The example app does not actually initialize the PostHog client during prerendering because the initialization is wrapped in a useEffect.

@danilofuchs's reproduction passes the API key to PostHogProvider, and PostHogProvider initializes the PostHog client in a useMemo. This means posthog-js's client gets initialized during prerendering on the server.

I tracked down PR #705 where the client initialization was moved from a useEffect to a useMemov1.68.3. Though that change did not visibly break things, it does break a major rule of useMemo:

You should only rely on useMemo as a performance optimization. If your code doesn’t work without it, find the underlying problem and fix it first. Then you may add useMemo to improve performance.

That PR is going to need to be reverted if people are going to be able to use posthog-js/react to initialize the PostHog client for them in apps that make use of server-side prerendering. Even in apps that do not server-side prerender, I would not be comfortable having posthog-js/react initialize the PostHog client because it is causing the component to have side effects during rendering (e.g. API requests, changing the state of the PostHog client). The correct place for those types of changes in React is within a useEffect, as the posthog-js client was doing in versions prior to v1.68.3.

Some potentially related tickets:

  1. 1079 and this ticket seem like duplicates of each other

  2. 1031 looks to also be related

  3. 723 I am not familiar with Gatsby, but I receive similar warnings in Next.js' prerendering to those described in issue 723

Moving the PostHog client init back to a useEffect will help all of these issues, if not completely solve them.

Until posthog-js/react gets fixed, my recommended workaround is to change your code to something like this @danilofuchs:

import { useUser } from "@clerk/nextjs";
import { PostHogProvider as Provider, usePostHog } from "posthog-js/react";
import { useEffect } from "react";
import { env } from "~/env.js";

interface Props {
  children: React.ReactNode;
}

export const PostHogProvider = ({ children }: Props) => {
  const [postHogClient, setPostHogClient] = useState<PostHog | undefined>();

  useEffect(() => {
    if (postHogClient) {
      return;
    }

    posthog.init(env.NEXT_PUBLIC_POSTHOG_KEY);
    setPostHogClient(posthog);
  }, []);

  return (
    <Provider client={postHogClient}>
      <PostHogUserSync />
      {children}
    </Provider>
  );
};

// ...

Thanks y'all for all you do!

danilofuchs commented 5 months ago

Great investigation @lightster!

I reviewed the official documentation for Pages Router and they recommend initializing in client only:

https://posthog.com/docs/libraries/next-js#pages-router

This solved the issue for us, but I believe PostHogProvider should initialize the SDK correctly out of the box