From e0dfd456695907d37763c76457a5f00b1bbf3da6 Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Thu, 13 Mar 2025 18:39:02 +0100 Subject: [PATCH] remove redundant engagement cooldown --- tracker/src/plausible.js | 15 +++------------ tracker/test/engagement.spec.js | 10 +--------- tracker/test/scroll-depth.spec.js | 4 +--- tracker/test/support/test-utils.js | 6 ------ 4 files changed, 5 insertions(+), 30 deletions(-) diff --git a/tracker/src/plausible.js b/tracker/src/plausible.js index c01713c9ebdf..868f072f6ca3 100644 --- a/tracker/src/plausible.js +++ b/tracker/src/plausible.js @@ -42,16 +42,10 @@ // prevents registering multiple listeners in those cases. var listeningOnEngagement = false - // In SPA-s, multiple listeners that trigger the engagement event - // might fire nearly at the same time. E.g. when navigating back - // in browser history while using hash-based routing - a popstate - // and hashchange will be fired in a very quick succession. This - // flag prevents sending multiple engagement events in those cases. - var engagementCooldown = false - // Timestamp indicating when this particular page last became visible. // Reset during pageviews, set to null when page is closed. var runningEngagementStart = null + // When page is hidden, this 'engaged' time is saved to this variable var currentEngagementTime = 0 @@ -121,13 +115,10 @@ The first engagement event is always sent due to containing at least the initial scroll depth. - We don't send engagements if: - - Less than 300ms have passed since the last engagement event - - The current pageview is ignored (onIgnoredEvent) + Also, we don't send engagements if the current pageview is ignored (onIgnoredEvent) */ - if (!engagementCooldown && !currentEngagementIgnored && (currentEngagementMaxScrollDepth < maxScrollDepthPx || engagementTime >= 3000)) { + if (!currentEngagementIgnored && (currentEngagementMaxScrollDepth < maxScrollDepthPx || engagementTime >= 3000)) { currentEngagementMaxScrollDepth = maxScrollDepthPx - setTimeout(function () {engagementCooldown = false}, 300) var payload = { n: 'engagement', diff --git a/tracker/test/engagement.spec.js b/tracker/test/engagement.spec.js index a6386c901a2e..37c7c7eb0267 100644 --- a/tracker/test/engagement.spec.js +++ b/tracker/test/engagement.spec.js @@ -1,5 +1,5 @@ const { expect } = require("@playwright/test") -const { expectPlausibleInAction, engagementCooldown, hideAndShowCurrentTab } = require('./support/test-utils') +const { expectPlausibleInAction, hideAndShowCurrentTab } = require('./support/test-utils') const { test } = require('@playwright/test') const { LOCAL_SERVER_ADDR } = require('./support/server') @@ -99,8 +99,6 @@ test.describe('engagement events', () => { expectedRequests: [{n: 'engagement', u: pageBaseURL, h: 1}] }) - await engagementCooldown(page) - // Navigate from ignored page to a tracked page -> // no engagement from the current page, pageview on the next page await expectPlausibleInAction(page, { @@ -109,8 +107,6 @@ test.describe('engagement events', () => { refutedRequests: [{n: 'engagement'}] }) - await engagementCooldown(page) - // Navigate from a tracked page to another tracked page -> // engagement with the last page URL, pageview with the new URL await expectPlausibleInAction(page, { @@ -162,8 +158,6 @@ test.describe('engagement events', () => { ] }) - await engagementCooldown(page) - await expectPlausibleInAction(page, { action: () => page.click('#jane-post'), expectedRequests: [ @@ -172,8 +166,6 @@ test.describe('engagement events', () => { ] }) - await engagementCooldown(page) - await expectPlausibleInAction(page, { action: () => page.click('#home'), expectedRequests: [ diff --git a/tracker/test/scroll-depth.spec.js b/tracker/test/scroll-depth.spec.js index 3d94fabbba65..d33019c795fb 100644 --- a/tracker/test/scroll-depth.spec.js +++ b/tracker/test/scroll-depth.spec.js @@ -1,4 +1,4 @@ -const { engagementCooldown, expectPlausibleInAction, hideCurrentTab, hideAndShowCurrentTab } = require('./support/test-utils') +const { expectPlausibleInAction, hideCurrentTab, hideAndShowCurrentTab } = require('./support/test-utils') const { test } = require('@playwright/test') const { LOCAL_SERVER_ADDR } = require('./support/server') @@ -32,8 +32,6 @@ test.describe('scroll depth (engagement events)', () => { ] }) - await engagementCooldown(page) - await expectPlausibleInAction(page, { action: () => page.click('#home-link'), expectedRequests: [ diff --git a/tracker/test/support/test-utils.js b/tracker/test/support/test-utils.js index f6f8d7cd4895..c675e6a3683d 100644 --- a/tracker/test/support/test-utils.js +++ b/tracker/test/support/test-utils.js @@ -1,11 +1,5 @@ const { expect, Page } = require("@playwright/test"); -// Since engagement events in the Plausible script are throttled to 300ms, we -// often need to wait for an artificial timeout before navigating in tests. -exports.engagementCooldown = async function(page) { - return page.waitForTimeout(400) -} - // Mocks an HTTP request call with the given path. Returns a Promise that resolves to the request // data. If the request is not made, resolves to null after 3 seconds. const mockRequest = function (page, path) {