From ae92c71f9f5b1c48bdb94312f1cf564d900abc0b Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Mon, 16 Dec 2024 15:00:24 -0300 Subject: [PATCH] chore: Make `fetch` our default request implementation `fetch` is much more optimized in modern browsers, there shouldn't be any reason not to use it in place of `XMLHTTPRequest` as it's heavily supported. Also, we're still keeping the `XML` fallback if `fetch` doesn't exist. Closes #1487 --- cypress/e2e/session-recording.cy.ts | 23 ++++++++++++----------- src/__tests__/featureflags.test.ts | 2 -- src/__tests__/surveys.test.ts | 2 -- src/posthog-core.ts | 10 +++++----- src/posthog-featureflags.ts | 1 - src/posthog-surveys.ts | 1 - src/request.ts | 15 +++++++-------- src/web-experiments.ts | 1 - 8 files changed, 24 insertions(+), 31 deletions(-) diff --git a/cypress/e2e/session-recording.cy.ts b/cypress/e2e/session-recording.cy.ts index 0875c32dd..108e0c670 100644 --- a/cypress/e2e/session-recording.cy.ts +++ b/cypress/e2e/session-recording.cy.ts @@ -249,13 +249,13 @@ describe('Session recording', () => { [/http:\/\/localhost:\d+\/static\/array.js/, 'script'], [ /http:\/\/localhost:\d+\/decide\/\?v=3&ip=1&_=\d+&ver=1\.\d\d\d\.\d+&compression=base64/, - 'xmlhttprequest', + 'lulul1', ], [/http:\/\/localhost:\d+\/static\/recorder.js\?v=1\.\d\d\d\.\d+/, 'script'], - [/https:\/\/example.com/, 'fetch'], + [/https:\/\/example.com/, 'lulul2'], ] - // yay, includes expected type 6 network data + // yay, includes expected network data expect(capturedRequests.length).to.equal(expectedCaptureds.length) expectedCaptureds.forEach(([url, initiatorType], index) => { expect(capturedRequests[index].name).to.match(url) @@ -269,7 +269,7 @@ describe('Session recording', () => { expect(capturedFetchRequest.fetchStart).to.be.greaterThan(0) // proxy for including network timing info - expect(capturedFetchRequest.initiatorType).to.eql('fetch') + expect(capturedFetchRequest.initiatorType).to.eql('lulul3') expect(capturedFetchRequest.isInitial).to.be.undefined expect(capturedFetchRequest.requestBody).to.eq('i am the fetch body') @@ -281,7 +281,7 @@ describe('Session recording', () => { }) }) - it('it captures XHR method correctly', () => { + it('it captures xhr methods correctly', () => { cy.get('[data-cy-xhr-call-button]').click() cy.wait('@example.com') cy.wait('@session-recording') @@ -304,17 +304,19 @@ describe('Session recording', () => { [/http:\/\/localhost:\d+\/static\/array.js/, 'script'], [ /http:\/\/localhost:\d+\/decide\/\?v=3&ip=1&_=\d+&ver=1\.\d\d\d\.\d+&compression=base64/, - 'xmlhttprequest', + 'lulul5', ], [/http:\/\/localhost:\d+\/static\/recorder.js\?v=1\.\d\d\d\.\d+/, 'script'], - [/https:\/\/example.com/, 'xmlhttprequest'], + [/https:\/\/example.com/, 'lulul6'], ] - // yay, includes expected type 6 network data + // yay, includes expected network data expect(capturedRequests.length).to.equal(expectedCaptureds.length) expectedCaptureds.forEach(([url, initiatorType], index) => { - expect(capturedRequests[index].name).to.match(url) - expect(capturedRequests[index].initiatorType).to.equal(initiatorType) + const capturedRequest = capturedRequests[index] + + expect(capturedRequest.name).to.match(url) + expect(capturedRequest.initiatorType).to.equal(initiatorType) }) // the HTML file that cypress is operating on (playground/cypress/index.html) @@ -324,7 +326,6 @@ describe('Session recording', () => { expect(capturedFetchRequest.fetchStart).to.be.greaterThan(0) // proxy for including network timing info - expect(capturedFetchRequest.initiatorType).to.eql('xmlhttprequest') expect(capturedFetchRequest.method).to.eql('POST') expect(capturedFetchRequest.isInitial).to.be.undefined expect(capturedFetchRequest.requestBody).to.eq('i am the xhr body') diff --git a/src/__tests__/featureflags.test.ts b/src/__tests__/featureflags.test.ts index a54ada8fd..ebf0084f6 100644 --- a/src/__tests__/featureflags.test.ts +++ b/src/__tests__/featureflags.test.ts @@ -475,7 +475,6 @@ describe('featureflags', () => { expect(instance._send_request).toHaveBeenCalledWith({ url: 'https://us.i.posthog.com/api/early_access_features/?token=random fake token', method: 'GET', - transport: 'XHR', callback: expect.any(Function), }) expect(instance._send_request).toHaveBeenCalledTimes(1) @@ -507,7 +506,6 @@ describe('featureflags', () => { url: 'https://us.i.posthog.com/api/early_access_features/?token=random fake token', method: 'GET', callback: expect.any(Function), - transport: 'XHR', }) expect(instance._send_request).toHaveBeenCalledTimes(1) diff --git a/src/__tests__/surveys.test.ts b/src/__tests__/surveys.test.ts index 6a4fa6916..1a465f73d 100644 --- a/src/__tests__/surveys.test.ts +++ b/src/__tests__/surveys.test.ts @@ -236,7 +236,6 @@ describe('surveys', () => { expect(instance._send_request).toHaveBeenCalledWith({ url: 'https://us.i.posthog.com/api/surveys/?token=testtoken', method: 'GET', - transport: 'XHR', callback: expect.any(Function), }) expect(instance._send_request).toHaveBeenCalledTimes(1) @@ -281,7 +280,6 @@ describe('surveys', () => { expect(instance._send_request).toHaveBeenCalledWith({ url: 'https://us.i.posthog.com/api/surveys/?token=testtoken', method: 'GET', - transport: 'XHR', callback: expect.any(Function), }) expect(instance._send_request).toHaveBeenCalledTimes(1) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 725a98daf..1968d0e24 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -575,8 +575,8 @@ export class PostHog { this.compression = includes(config['supportedCompression'], Compression.GZipJS) ? Compression.GZipJS : includes(config['supportedCompression'], Compression.Base64) - ? Compression.Base64 - : undefined + ? Compression.Base64 + : undefined } if (config.analytics?.endpoint) { @@ -587,8 +587,8 @@ export class PostHog { person_profiles: this._initialPersonProfilesConfig ? this._initialPersonProfilesConfig : config['defaultIdentifiedOnly'] - ? 'identified_only' - : 'always', + ? 'identified_only' + : 'always', }) this.siteApps?.onRemoteConfig(config) @@ -1710,7 +1710,7 @@ export class PostHog { * // Capture rage clicks * rageclick: true * - * // transport for sending requests ('XHR' or 'sendBeacon') + * // transport for sending requests ('XHR' | 'fetch' | 'sendBeacon') * // NB: sendBeacon should only be used for scenarios such as * // page unload where a "best-effort" attempt to send is * // acceptable; the sendBeacon API does not support callbacks diff --git a/src/posthog-featureflags.ts b/src/posthog-featureflags.ts index e8d2e9c3e..250bde8b1 100644 --- a/src/posthog-featureflags.ts +++ b/src/posthog-featureflags.ts @@ -433,7 +433,6 @@ export class PostHogFeatureFlags { if (!existing_early_access_features || force_reload) { this.instance._send_request({ - transport: 'XHR', url: this.instance.requestRouter.endpointFor( 'api', `/api/early_access_features/?token=${this.instance.config.token}` diff --git a/src/posthog-surveys.ts b/src/posthog-surveys.ts index f33fe4f4a..3d3c8a488 100644 --- a/src/posthog-surveys.ts +++ b/src/posthog-surveys.ts @@ -118,7 +118,6 @@ export class PostHogSurveys { `/api/surveys/?token=${this.instance.config.token}` ), method: 'GET', - transport: 'XHR', callback: (response) => { if (response.statusCode !== 200 || !response.json) { return callback([]) diff --git a/src/request.ts b/src/request.ts index ea4ab1343..d8ec9f4f0 100644 --- a/src/request.ts +++ b/src/request.ts @@ -216,18 +216,17 @@ const _sendBeacon = (options: RequestOptions) => { const AVAILABLE_TRANSPORTS: { transport: RequestOptions['transport']; method: (options: RequestOptions) => void }[] = [] // We add the transports in order of preference - -if (XMLHttpRequest) { +if (fetch) { AVAILABLE_TRANSPORTS.push({ - transport: 'XHR', - method: xhr, + transport: 'fetch', + method: _fetch, }) } -if (fetch) { +if (XMLHttpRequest) { AVAILABLE_TRANSPORTS.push({ - transport: 'fetch', - method: _fetch, + transport: 'XHR', + method: xhr, }) } @@ -250,7 +249,7 @@ export const request = (_options: RequestOptions) => { compression: options.compression, }) - const transport = options.transport ?? 'XHR' + const transport = options.transport ?? 'fetch' const transportMethod = find(AVAILABLE_TRANSPORTS, (t) => t.transport === transport)?.method ?? AVAILABLE_TRANSPORTS[0].method diff --git a/src/web-experiments.ts b/src/web-experiments.ts index 74bbc904b..eed4b0ca0 100644 --- a/src/web-experiments.ts +++ b/src/web-experiments.ts @@ -151,7 +151,6 @@ export class WebExperiments { `/api/web_experiments/?token=${this.instance.config.token}` ), method: 'GET', - transport: 'XHR', callback: (response) => { if (response.statusCode !== 200 || !response.json) { return callback([])