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

chore: remove v1 rrweb loading #1078

Merged
merged 5 commits into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cypress/support/e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ beforeEach(() => {

cy.readFile('dist/recorder.js').then((body) => {
cy.intercept('**/static/recorder.js*', { body }).as('recorder')
cy.intercept('**/static/recorder-v2.js*', { body }).as('recorder')
benjackwhite marked this conversation as resolved.
Show resolved Hide resolved
})

cy.readFile('dist/surveys.js').then((body) => {
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@
"rollup-plugin-visualizer": "^5.12.0",
"rrweb": "2.0.0-alpha.11",
"rrweb-snapshot": "2.0.0-alpha.11",
"rrweb-v1": "npm:[email protected]",
"sinon": "9.0.2",
"testcafe": "1.19.0",
"testcafe-browser-provider-browserstack": "1.14.0",
Expand Down
22 changes: 0 additions & 22 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 1 addition & 6 deletions rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,8 @@ export default [
format: 'iife',
name: 'posthog',
},
],
plugins: [...plugins],
},
{
input: 'src/loader-recorder-v2.ts',
output: [
{
// Backwards compatibility for older SDK versions
file: 'dist/recorder-v2.js',
sourcemap: true,
format: 'iife',
Expand Down
39 changes: 0 additions & 39 deletions src/__tests__/extensions/replay/sessionrecording.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
SESSION_RECORDING_CANVAS_RECORDING,
SESSION_RECORDING_ENABLED_SERVER_SIDE,
SESSION_RECORDING_IS_SAMPLED,
SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE,
} from '../../../constants'
import { SessionIdManager } from '../../../sessionid'
import {
Expand Down Expand Up @@ -145,7 +144,6 @@ describe('SessionRecording', () => {
// defaults
posthog.persistence?.register({
[SESSION_RECORDING_ENABLED_SERVER_SIDE]: true,
[SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE]: 'v2',
[CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE]: false,
[SESSION_RECORDING_IS_SAMPLED]: undefined,
})
Expand Down Expand Up @@ -193,33 +191,6 @@ describe('SessionRecording', () => {
})
})

describe('getRecordingVersion', () => {
it('uses client side setting v2 over server side', () => {
posthog.persistence?.register({ [SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE]: 'v1' })
posthog.config.session_recording.recorderVersion = 'v2'
expect(sessionRecording['recordingVersion']).toBe('v2')
})

it('uses client side setting v1 over server side', () => {
posthog.persistence?.register({ [SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE]: 'v2' })
posthog.config.session_recording.recorderVersion = 'v1'
expect(sessionRecording['recordingVersion']).toBe('v1')
})

it('uses server side setting if client side setting is not set', () => {
posthog.config.session_recording.recorderVersion = undefined

posthog.persistence?.register({ [SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE]: 'v1' })
expect(sessionRecording['recordingVersion']).toBe('v1')

posthog.persistence?.register({ [SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE]: 'v2' })
expect(sessionRecording['recordingVersion']).toBe('v2')

posthog.persistence?.register({ [SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE]: undefined })
expect(sessionRecording['recordingVersion']).toBe('v1')
})
})

describe('startRecordingIfEnabled', () => {
beforeEach(() => {
// need to cast as any to mock private methods
Expand Down Expand Up @@ -719,16 +690,7 @@ describe('SessionRecording', () => {
expect(loadScript).not.toHaveBeenCalled()
})

it('loads recording v1 script from right place', () => {
posthog.config.session_recording.recorderVersion = 'v1'

sessionRecording.startRecordingIfEnabled()

expect(loadScript).toHaveBeenCalledWith('https://test.com/static/recorder.js?v=v0.0.1', expect.anything())
})

it('loads recording v2 script from right place', () => {
posthog.persistence?.register({ [SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE]: 'v2' })
sessionRecording.startRecordingIfEnabled()

expect(loadScript).toHaveBeenCalledWith(
Expand All @@ -739,7 +701,6 @@ describe('SessionRecording', () => {

it('load correct recording version if there is a cached mismatch', () => {
posthog.__loaded_recorder_version = 'v1'
posthog.persistence?.register({ [SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE]: 'v2' })
sessionRecording.startRecordingIfEnabled()

expect(loadScript).toHaveBeenCalledWith(
Expand Down
1 change: 0 additions & 1 deletion src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ export const EVENT_TIMERS_KEY = '__timers'
export const AUTOCAPTURE_DISABLED_SERVER_SIDE = '$autocapture_disabled_server_side'
export const SESSION_RECORDING_ENABLED_SERVER_SIDE = '$session_recording_enabled_server_side'
export const CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE = '$console_log_recording_enabled_server_side'
export const SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE = '$session_recording_recorder_version_server_side' // follows rrweb versioning
export const SESSION_RECORDING_NETWORK_PAYLOAD_CAPTURE = '$session_recording_network_payload_capture'
export const SESSION_RECORDING_CANVAS_RECORDING = '$session_recording_canvas_recording'
export const SESSION_ID = '$sesid'
Expand Down
16 changes: 3 additions & 13 deletions src/extensions/replay/sessionrecording.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
SESSION_RECORDING_ENABLED_SERVER_SIDE,
SESSION_RECORDING_IS_SAMPLED,
SESSION_RECORDING_NETWORK_PAYLOAD_CAPTURE,
SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE,
} from '../../constants'
import {
FULL_SNAPSHOT_EVENT_TYPE,
Expand Down Expand Up @@ -192,12 +191,6 @@ export class SessionRecording {
: undefined
}

private get recordingVersion() {
const recordingVersion_server_side = this.instance.get_property(SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE)
const recordingVersion_client_side = this.instance.config.session_recording?.recorderVersion
return recordingVersion_client_side || recordingVersion_server_side || 'v1'
}

// network payload capture config has three parts
// each can be configured server side or client side
private get networkPayloadCapture():
Expand Down Expand Up @@ -341,7 +334,6 @@ export class SessionRecording {
this.instance.persistence.register({
[SESSION_RECORDING_ENABLED_SERVER_SIDE]: !!response['sessionRecording'],
[CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE]: response.sessionRecording?.consoleLogRecordingEnabled,
[SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE]: response.sessionRecording?.recorderVersion,
[SESSION_RECORDING_NETWORK_PAYLOAD_CAPTURE]: {
capturePerformance: response.capturePerformance,
...response.sessionRecording?.networkPayloadCapture,
Expand Down Expand Up @@ -429,17 +421,15 @@ export class SessionRecording {
// We want to ensure the sessionManager is reset if necessary on load of the recorder
this.sessionManager.checkAndGetSessionAndWindowId()

const recorderJS = this.recordingVersion === 'v2' ? 'recorder-v2.js' : 'recorder.js'

// If recorder.js is already loaded (if array.full.js snippet is used or posthog-js/dist/recorder is
// imported) or matches the requested recorder version, don't load script. Otherwise, remotely import
// recorder.js from cdn since it hasn't been loaded.
if (this.instance.__loaded_recorder_version !== this.recordingVersion) {
if (this.instance.__loaded_recorder_version !== 'v2') {
loadScript(
this.instance.requestRouter.endpointFor('assets', `/static/${recorderJS}?v=${Config.LIB_VERSION}`),
this.instance.requestRouter.endpointFor('assets', `/static/recorder-v2.js?v=${Config.LIB_VERSION}`),
(err) => {
if (err) {
return logger.error(LOGGER_PREFIX + ` could not load ${recorderJS}`, err)
return logger.error(LOGGER_PREFIX + ` could not load recorder-v2.js`, err)
}

this._onScriptLoaded()
Expand Down
Loading
Loading