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

fix: support for SSR #57

Merged
merged 3 commits into from
Jan 16, 2023
Merged

fix: support for SSR #57

merged 3 commits into from
Jan 16, 2023

Conversation

paolodamico
Copy link
Contributor

This PR refactors the code posthog-web so the library can be imported in Next.js projects with SSR. Instead of window having to be defined just from importing the library, this relies on window being defined on the constructor (i.e. when an object is initialized).

Before in a Next.JS project

import PostHog from 'posthog-js-lite' // <- this would generate an exception because window is not defined

After (this works)

import PostHog from 'posthog-js-lite';

if (typeof window !== 'undefined') {
    posthog = new PostHog(
        'phc_my_key',
    )
}

CC @m1guelpf

@paolodamico
Copy link
Contributor Author

In addition, we also get rid of package-lock.json to avoid having two duplicate lock files which can cause confusion or maintenance issues.

@timgl timgl added the bump patch Bump patch version when this PR gets merged label Jan 16, 2023
Copy link
Collaborator

@benjackwhite benjackwhite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great solution, nicely implemented!

Thanks @paolodamico !

@benjackwhite benjackwhite merged commit a7f2ff8 into PostHog:master Jan 16, 2023
@paolodamico paolodamico deleted the fix-undefined branch January 16, 2023 17:18
@paolodamico
Copy link
Contributor Author

thx @benjackwhite, appreciate the super quick turnaround! could we trouble you with pushing a release so we can ship our app?

@@ -19,7 +19,7 @@ export class PostHog extends PostHogCore {

// posthog-js stores options in one object on
this._storageKey = options?.persistence_name ? `ph_${options.persistence_name}` : `ph_${apiKey}_posthog`
this._storage = getStorage(options?.persistence || 'localStorage')
this._storage = getStorage(options?.persistence || 'localStorage', window)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still needs to be gated because it is still throwing a ReferenceError – you can't just reference window in the node.js global scope.

this._storage = getStorage(options?.persistence || 'localStorage', typeof window !== 'undefined' ? window : undefined)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump patch Bump patch version when this PR gets merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants