From 8a910c57d4157bdd06f727bb06f638049b66d28a Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 1 Feb 2024 12:11:53 +0100 Subject: [PATCH] feat: Fetch without window (#985) --- src/__tests__/send-request.test.ts | 18 +++++++++++----- src/__tests__/surveys.test.ts | 34 +++++++++++++++--------------- src/send-request.ts | 19 ++++++++--------- src/session-props.ts | 4 ++-- src/utils/event-utils.ts | 8 +++---- src/utils/globals.ts | 24 ++++++++++++++++----- src/utils/request-utils.ts | 10 ++++----- 7 files changed, 69 insertions(+), 48 deletions(-) diff --git a/src/__tests__/send-request.test.ts b/src/__tests__/send-request.test.ts index 176fcf3df..23105367e 100644 --- a/src/__tests__/send-request.test.ts +++ b/src/__tests__/send-request.test.ts @@ -6,7 +6,6 @@ import { assert, boolean, property, uint8Array, VerbosityLevel } from 'fast-chec import { Compression, PostData, XHROptions, RequestData, MinimalHTTPResponse } from '../types' import { _isUndefined } from '../utils/type-utils' -import { assignableWindow } from '../utils/globals' jest.mock('../utils/request-utils', () => ({ ...jest.requireActual('../utils/request-utils'), @@ -15,6 +14,15 @@ jest.mock('../utils/request-utils', () => ({ SUPPORTS_REQUEST: true, })) +jest.mock('../utils/globals', () => ({ + ...jest.requireActual('../utils/globals'), + fetch: jest.fn(), +})) + +import { assignableWindow, fetch } from '../utils/globals' + +const mockedFetch = fetch as jest.MockedFunction + jest.mock('../config', () => ({ DEBUG: false, LIB_VERSION: '1.23.45' })) const flushPromises = () => new Promise((r) => setTimeout(r, 0)) @@ -173,7 +181,7 @@ describe('send-request', () => { } } - assignableWindow.fetch = jest.fn(() => { + mockedFetch.mockImplementation(() => { return Promise.resolve({ status: 200, json: () => Promise.resolve({}), @@ -190,7 +198,7 @@ describe('send-request', () => { }) ) - expect(assignableWindow.fetch).toHaveBeenCalledWith( + expect(mockedFetch).toHaveBeenCalledWith( `https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278`, { body: null, @@ -209,7 +217,7 @@ describe('send-request', () => { url: 'https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278', }) ) - expect(assignableWindow.fetch).toHaveBeenCalledWith( + expect(mockedFetch).toHaveBeenCalledWith( `https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278&retry_count=${retryCount}`, { body: null, @@ -231,7 +239,7 @@ describe('send-request', () => { describe('when the requests fail', () => { beforeEach(() => { - assignableWindow.fetch = jest.fn( + mockedFetch.mockImplementation( () => Promise.resolve({ status: 502, diff --git a/src/__tests__/surveys.test.ts b/src/__tests__/surveys.test.ts index 301c934a1..8f0896bb1 100644 --- a/src/__tests__/surveys.test.ts +++ b/src/__tests__/surveys.test.ts @@ -5,14 +5,14 @@ import { SurveyType, SurveyQuestionType, Survey } from '../posthog-surveys-types import { PostHogPersistence } from '../posthog-persistence' import { PostHog } from '../posthog-core' import { DecideResponse, PostHogConfig, Properties } from '../types' -import { window } from '../utils/globals' +import { assignableWindow } from '../utils/globals' describe('surveys', () => { let config: PostHogConfig let instance: PostHog let surveys: PostHogSurveys let surveysResponse: { status?: number; surveys?: Survey[] } - const originalWindowLocation = window!.location + const originalWindowLocation = assignableWindow.location const decideResponse = { featureFlags: { @@ -286,11 +286,11 @@ describe('surveys', () => { surveys: [surveyWithUrl, surveyWithSelector, surveyWithUrlAndSelector], } // eslint-disable-next-line compat/compat - window!.location = new URL('https://posthog.com') as unknown as Location + assignableWindow.location = new URL('https://posthog.com') as unknown as Location surveys.getActiveMatchingSurveys((data) => { expect(data).toEqual([surveyWithUrl]) }) - window!.location = originalWindowLocation + assignableWindow.location = originalWindowLocation document.body.appendChild(document.createElement('div')).className = 'test-selector' surveys.getActiveMatchingSurveys((data) => { @@ -302,7 +302,7 @@ describe('surveys', () => { } // eslint-disable-next-line compat/compat - window!.location = new URL('https://posthogapp.com') as unknown as Location + assignableWindow.location = new URL('https://posthogapp.com') as unknown as Location document.body.appendChild(document.createElement('div')).id = 'foo' surveys.getActiveMatchingSurveys((data) => { @@ -325,41 +325,41 @@ describe('surveys', () => { ], } - const originalWindowLocation = window!.location + const originalWindowLocation = assignableWindow.location // eslint-disable-next-line compat/compat - window!.location = new URL('https://regex-url.com/test') as unknown as Location + assignableWindow.location = new URL('https://regex-url.com/test') as unknown as Location surveys.getActiveMatchingSurveys((data) => { expect(data).toEqual([surveyWithRegexUrl]) }) - window!.location = originalWindowLocation + assignableWindow.location = originalWindowLocation // eslint-disable-next-line compat/compat - window!.location = new URL('https://example.com?name=something') as unknown as Location + assignableWindow.location = new URL('https://example.com?name=something') as unknown as Location surveys.getActiveMatchingSurveys((data) => { expect(data).toEqual([surveyWithParamRegexUrl]) }) - window!.location = originalWindowLocation + assignableWindow.location = originalWindowLocation // eslint-disable-next-line compat/compat - window!.location = new URL('https://app.subdomain.com') as unknown as Location + assignableWindow.location = new URL('https://app.subdomain.com') as unknown as Location surveys.getActiveMatchingSurveys((data) => { expect(data).toEqual([surveyWithWildcardSubdomainUrl]) }) - window!.location = originalWindowLocation + assignableWindow.location = originalWindowLocation // eslint-disable-next-line compat/compat - window!.location = new URL('https://wildcard.com/something/other') as unknown as Location + assignableWindow.location = new URL('https://wildcard.com/something/other') as unknown as Location surveys.getActiveMatchingSurveys((data) => { expect(data).toEqual([surveyWithWildcardRouteUrl]) }) - window!.location = originalWindowLocation + assignableWindow.location = originalWindowLocation // eslint-disable-next-line compat/compat - window!.location = new URL('https://example.com/exact') as unknown as Location + assignableWindow.location = new URL('https://example.com/exact') as unknown as Location surveys.getActiveMatchingSurveys((data) => { expect(data).toEqual([surveyWithExactUrlMatch]) }) - window!.location = originalWindowLocation + assignableWindow.location = originalWindowLocation }) it('returns surveys that match linked and targeting feature flags', () => { @@ -379,7 +379,7 @@ describe('surveys', () => { it('returns surveys that inclusively matches any of the above', () => { // eslint-disable-next-line compat/compat - window!.location = new URL('https://posthogapp.com') as unknown as Location + assignableWindow.location = new URL('https://posthogapp.com') as unknown as Location document.body.appendChild(document.createElement('div')).className = 'test-selector' surveysResponse = { surveys: [activeSurvey, surveyWithSelector, surveyWithEverything] } // activeSurvey returns because there are no restrictions on conditions or flags on it diff --git a/src/send-request.ts b/src/send-request.ts index 15508a9ea..7d58eefdd 100644 --- a/src/send-request.ts +++ b/src/send-request.ts @@ -1,11 +1,11 @@ import { _each } from './utils' import Config from './config' import { PostData, XHROptions, RequestData, MinimalHTTPResponse } from './types' -import { SUPPORTS_FETCH, _HTTPBuildQuery } from './utils/request-utils' +import { _HTTPBuildQuery } from './utils/request-utils' import { _isArray, _isFunction, _isNumber, _isUint8Array, _isUndefined } from './utils/type-utils' import { logger } from './utils/logger' -import { window } from './utils/globals' +import { fetch } from './utils/globals' export const addParamsToURL = ( url: string, @@ -64,7 +64,7 @@ export const encodePostData = (data: PostData | Uint8Array, options: Partial { // NOTE: Until we are confident with it, we only use fetch if explicitly told so - if (window && SUPPORTS_FETCH && params.options.transport === 'fetch') { + if (fetch && params.options.transport === 'fetch') { const body = encodePostData(params.data, params.options) const headers = new Headers() @@ -82,13 +82,12 @@ export const request = (params: RequestData) => { url = addParamsToURL(url, { retry_count: params.retriesPerformedSoFar }, {}) } - window - .fetch(url, { - method: params.options?.method || 'GET', - headers, - keepalive: params.options.method === 'POST', - body, - }) + fetch(url, { + method: params.options?.method || 'GET', + headers, + keepalive: params.options.method === 'POST', + body, + }) .then((response) => { const statusCode = response.status // Report to the callback handlers diff --git a/src/session-props.ts b/src/session-props.ts index 79021ae80..3a4fb385d 100644 --- a/src/session-props.ts +++ b/src/session-props.ts @@ -6,7 +6,7 @@ * * These have the same lifespan as a session_id */ -import { window } from './utils/globals' +import { location } from './utils/globals' import { _info } from './utils/event-utils' import { SessionIdManager } from './sessionid' import { PostHogPersistence } from './posthog-persistence' @@ -29,7 +29,7 @@ interface StoredSessionSourceProps { const generateSessionSourceParams = (): SessionSourceProps => { return { - initialPathName: window?.location.pathname || '', + initialPathName: location?.pathname || '', referringDomain: _info.referringDomain(), ..._info.campaignParams(), } diff --git a/src/utils/event-utils.ts b/src/utils/event-utils.ts index ba6e9a0cd..717cdc66a 100644 --- a/src/utils/event-utils.ts +++ b/src/utils/event-utils.ts @@ -3,7 +3,7 @@ import { _isNull, _isUndefined } from './type-utils' import { Properties } from '../types' import Config from '../config' import { _each, _extend, _includes, _strip_empty_properties, _timestamp } from './index' -import { document, window, userAgent, assignableWindow } from './globals' +import { document, window, location, userAgent, assignableWindow } from './globals' /** * Safari detection turns out to be complicted. For e.g. https://stackoverflow.com/a/29696509 @@ -277,9 +277,9 @@ export const _info = { $device_type: _info.deviceType(userAgent), }), { - $current_url: window?.location.href, - $host: window?.location.host, - $pathname: window?.location.pathname, + $current_url: location?.href, + $host: location?.host, + $pathname: location?.pathname, $raw_user_agent: userAgent.length > 1000 ? userAgent.substring(0, 997) + '...' : userAgent, $browser_version: _info.browserVersion(userAgent, navigator.vendor, assignableWindow.opera), $browser_language: _info.browserLanguage(), diff --git a/src/utils/globals.ts b/src/utils/globals.ts index d90d41708..b0e5bfc54 100644 --- a/src/utils/globals.ts +++ b/src/utils/globals.ts @@ -1,13 +1,27 @@ /* - * Saved references to long variable names, so that bundling can minimize file size. + * Global helpers to protect access to browser globals in a way that is safer for different targets + * like DOM, SSR, Web workers etc. + * + * NOTE: Typically we want the "window" but globalThis works for both the typical browser context as + * well as other contexts such as the web worker context. Window is still exported for any bits that explicitly require it. + * If in doubt - export the global you need from this file and use that as an optional value. This way the code path is forced + * to handle the case where the global is not available. */ + +// eslint-disable-next-line no-restricted-globals +const win: (Window & typeof globalThis) | undefined = typeof window !== 'undefined' ? window : undefined + +const global: typeof globalThis | undefined = typeof globalThis !== 'undefined' ? globalThis : win + export const ArrayProto = Array.prototype export const nativeForEach = ArrayProto.forEach export const nativeIndexOf = ArrayProto.indexOf -// eslint-disable-next-line no-restricted-globals -export const win: (Window & typeof globalThis) | undefined = typeof window !== 'undefined' ? window : undefined -const navigator = win?.navigator -export const document = win?.document + +const navigator = global?.navigator +export const document = global?.document +export const location = global?.location +export const fetch = global?.fetch +export const XMLHttpRequest = global?.XMLHttpRequest export const userAgent = navigator?.userAgent export const assignableWindow: Window & typeof globalThis & Record = win ?? ({} as any) diff --git a/src/utils/request-utils.ts b/src/utils/request-utils.ts index 1acf562e2..3f6085361 100644 --- a/src/utils/request-utils.ts +++ b/src/utils/request-utils.ts @@ -1,14 +1,14 @@ import { _each, _isValidRegex } from './' -import { _isArray, _isFunction, _isUndefined } from './type-utils' +import { _isArray, _isUndefined } from './type-utils' import { logger } from './logger' -import { document, window } from './globals' +import { document, fetch, XMLHttpRequest } from './globals' const localDomains = ['localhost', '127.0.0.1'] -export const SUPPORTS_XHR = !!(window?.XMLHttpRequest && 'withCredentials' in new XMLHttpRequest()) -export const SUPPORTS_FETCH = !!(window?.fetch && _isFunction(window?.fetch)) -export const SUPPORTS_REQUEST = SUPPORTS_XHR || SUPPORTS_FETCH +export const SUPPORTS_XHR = !!(XMLHttpRequest && 'withCredentials' in new XMLHttpRequest()) +// eslint-disable-next-line compat/compat +export const SUPPORTS_REQUEST = SUPPORTS_XHR || !!fetch /** * IE11 doesn't support `new URL`