fix: check if window is available before using on web env #256
Conversation
|
Size Change: +281 B (+0.38%) Total Size: 73.8 kB
ℹ️ View Unchanged
|
| if (!window) { | ||
| return false | ||
| } |
There was a problem hiding this comment.
checkStoreIsSupported is only called if there's a window
| @@ -50,6 +56,9 @@ export class PostHog extends PostHogCore { | |||
| } | |||
|
|
|||
| fetch(url: string, options: PostHogFetchOptions): Promise<PostHogFetchResponse> { | |||
| // TODO: what to do here? | |||
| // should we move this to core? https://github.com/PostHog/posthog-js-lite/blob/main/posthog-node/src/fetch.ts | |||
| // and reuse it here? if window isn't available? | |||
There was a problem hiding this comment.
I don't really get what this means. How can we fetch if there is no window?
There was a problem hiding this comment.
In both Service Workers and Web Workers, there is no Window object, but the fetch() function is still available globally as part of the worker environment.
So I was assuming that https://github.com/PostHog/posthog-js-lite/blob/main/posthog-node/src/fetch.ts does something similar and we could reuse it here as a fallback if window isn't available.
Does that make sense?
There was a problem hiding this comment.
Ah i see. Yeah then I guess that same line would I think work for loading fetch from the global instead of window?
Might be a bit tricky to test...
There was a problem hiding this comment.
yep, that looks like the fix to me...
Co-authored-by: Ben White <[email protected]>
Co-authored-by: Ben White <[email protected]>
| const _window: Window | undefined = typeof window !== 'undefined' ? window : undefined | ||
| return _window |
There was a problem hiding this comment.
very nit-picky but _window can be in-lined here
| const _window: Window | undefined = typeof window !== 'undefined' ? window : undefined | |
| return _window | |
| return typeof window !== 'undefined' ? window : undefined |
| @@ -50,6 +56,9 @@ export class PostHog extends PostHogCore { | |||
| } | |||
|
|
|||
| fetch(url: string, options: PostHogFetchOptions): Promise<PostHogFetchResponse> { | |||
| // TODO: what to do here? | |||
| // should we move this to core? https://github.com/PostHog/posthog-js-lite/blob/main/posthog-node/src/fetch.ts | |||
| // and reuse it here? if window isn't available? | |||
There was a problem hiding this comment.
yep, that looks like the fix to me...
Problem
Closes #255
Similar to #57
Changes
Release info Sub-libraries affected
Bump level
Libraries affected
Changelog notes