-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
fix: Nextjs onboarding snippets #17979
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for doing this!!
export function PHProvider({ children }) { | ||
return <PostHogProvider client={posthog}>{children}</PostHogProvider> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've never used Next but it seems like this PHProvider
does nothing... is there a reason to create it instead of using PostHogProvider
directly? Does it have to be in a providers
file for some reason?
Further the PHProvider name confused me because I read it as PHP
and I thought "wait isn't this JS not PHP?" 😄 I'd recommend renaming the PostHogProvider
on import and using that name for this component if you are going to keep this component. But looking at this it's not defined anywhere...... how does that work? Is it Next magic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've never used Next but it seems like this PHProvider does nothing... is there a reason to create it instead of using PostHogProvider directly? Does it have to be in a providers file for some reason?
Basically the use client
directive makes it client-side render. It is similar to the typeof window check in the pages router. If you use it directly, it turns off server-side/static rendering all together and causes causes.
I forgot to import the PostHogProvider from the React component, I just changed the name so it isn't the same as the import. The PHProvider component is only used in that one spot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd still rename PHProvider to avoid confusion, something like CSPoshHogProvider for client side
Also someone said something about app vs pages router being more popular, I'm not sure which is which so if you can just make sure they are in the correct order (most popular first) that'd be 👍 |
* Fix nextjs onboarding snippets
Problem
Next.js feature flag onboarding snippets were missing a few key steps.
Changes
Added the steps and fixed a typo
How did you test this code?
Ran locally.