From 76dc1e31ac6925cbfaa40dad2d368dec059790b4 Mon Sep 17 00:00:00 2001 From: Conductor Adriaan Date: Mon, 19 Oct 2026 13:01:18 +0000 Subject: [PATCH] Run the pageview callback when the pageview is skipped sa_pageview(path, callback) only called the callback from the pixel's load or error handler, so a skipped pageview (a repeated path or a path excluded by ignore-pages) never ran it and code waiting for it hung. Call the callback on that early return as well, so it runs once per call, and cover repeated, ignored and namespaced calls in the unit tests. Sent pageviews keep calling the callback once from the pixel. The call is limited to builds with manual pageviews, so light.js is unchanged; the other scripts grow by 10 bytes minified and 1 to 2 bytes gzip. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/default.js | 5 ++ test/unit/pageview-callback.test.js | 103 ++++++++++++++++++++++------ 2 files changed, 87 insertions(+), 21 deletions(-) diff --git a/src/default.js b/src/default.js index 41ecf1c..0cffe03 100644 --- a/src/default.js +++ b/src/default.js @@ -844,7 +844,12 @@ // Don't send the last path again (this could happen when pushState is used to change the path hash or search) var pathKey = path + querySearch; + /** if (or spa hash) **/ + // Manual pageview callbacks also run when the pageview is skipped + if (!path || lastSendPath == pathKey) return callback(); + /** else **/ if (!path || lastSendPath == pathKey) return; + /** endif **/ lastSendPath = pathKey; page.path = path; diff --git a/test/unit/pageview-callback.test.js b/test/unit/pageview-callback.test.js index 6929b31..50bcb9a 100644 --- a/test/unit/pageview-callback.test.js +++ b/test/unit/pageview-callback.test.js @@ -1,33 +1,94 @@ const { expect } = require("chai"); const { createDOM } = require("./helpers/dom"); -describe("pageview callback", function () { - it("executes callback after sending a pageview", function (done) { - const dom = createDOM({ settings: { autoCollect: false } }); - - const sent = dom.sent; - dom.window.Image = function () { - return { - onload: null, - onerror: null, - set src(value) { - sent.push({ type: "image", url: value }); - if (this.onload) this.onload(); - }, - }; +function createLoadingDOM(settings) { + const dom = createDOM({ settings: { autoCollect: false, ...settings } }); + + const sent = dom.sent; + dom.window.Image = function () { + return { + onload: null, + onerror: null, + set src(value) { + sent.push({ type: "image", url: value }); + if (this.onload) this.onload(); + }, }; + }; + + return dom; +} - let called = false; +function sentPaths(dom) { + return dom.sent.map((r) => new URL(r.url).searchParams.get("path")); +} + +describe("pageview callback", function () { + it("executes callback once after sending a pageview", function (done) { + const dom = createLoadingDOM(); + + let calls = 0; dom.window.sa_pageview("/callback", function () { - called = true; + calls++; + }); + + setTimeout(() => { + expect(sentPaths(dom), "pageview requests").to.deep.equal(["/callback"]); + expect(calls, "callback calls").to.equal(1); + done(); + }, 10); + }); + + it("executes callback when a repeated path is skipped", function (done) { + const dom = createLoadingDOM(); + + const calls = [0, 0]; + dom.window.sa_pageview("/same", function () { + calls[0]++; + }); + dom.window.sa_pageview("/same", function () { + calls[1]++; + }); + + setTimeout(() => { + expect(sentPaths(dom), "pageview requests").to.deep.equal(["/same"]); + expect(calls, "callback calls").to.deep.equal([1, 1]); + done(); + }, 10); + }); + + it("executes callback when an ignored path is skipped", function (done) { + const dom = createLoadingDOM({ ignorePages: "/ignore" }); + + let calls = 0; + dom.window.sa_pageview("/ignore", { plan: "free" }, function () { + calls++; + }); + + setTimeout(() => { + expect(sentPaths(dom), "pageview requests").to.deep.equal([]); + expect(calls, "callback calls").to.equal(1); + done(); + }, 10); + }); + + it("executes callback when a namespaced pageview is skipped", function (done) { + const dom = createLoadingDOM({ namespace: "ba", ignorePages: "/ignore" }); + + const calls = [0, 0, 0]; + dom.window.ba_pageview("/same", function () { + calls[0]++; + }); + dom.window.ba_pageview("/same", { plan: "free" }, function () { + calls[1]++; + }); + dom.window.ba_pageview("/ignore", function () { + calls[2]++; }); setTimeout(() => { - const req = dom.sent.find( - (r) => r.type === "image" && /path=%2Fcallback/.test(r.url) - ); - expect(req, "pageview request").to.exist; - expect(called, "callback called").to.be.true; + expect(sentPaths(dom), "pageview requests").to.deep.equal(["/same"]); + expect(calls, "callback calls").to.deep.equal([1, 1, 1]); done(); }, 10); });