Skip to content

Commit e1062aa

Browse files
committed
fix: count back/forward revisits — key transitions, not key novelty
location.key is stable per history entry, so a POP restores the entry's original key; the persistent seen-keys set suppressed exactly the revisits the dedupe intended to count. Track only the current key and reset the per-page set on every key change: any navigation (push, replace, pop) counts again, while StrictMode re-mounts and co-located duplicates within one navigation still dedupe. Memory is now bounded to the current view instead of growing per session.
1 parent f20318f commit e1062aa

2 files changed

Lines changed: 29 additions & 13 deletions

File tree

packages/hydrogen/__tests__/use-pixel.test.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest'
22
import { shouldFirePixel } from '../src/utils/pixel'
33

44
describe('shouldFirePixel', () => {
5-
it('should_fire_once_per_page_per_navigation_entry', () => {
5+
it('should_fire_once_per_page_per_navigation', () => {
66
expect(shouldFirePixel('nav-1', 'page-a')).toBe(true)
77
// Second co-located instance of the same page (or a StrictMode
88
// re-mount) must not double-count the impression.
@@ -12,11 +12,17 @@ describe('shouldFirePixel', () => {
1212
it('should_count_each_co_located_page_separately', () => {
1313
expect(shouldFirePixel('nav-2', 'page-layout')).toBe(true)
1414
expect(shouldFirePixel('nav-2', 'page-child')).toBe(true)
15+
expect(shouldFirePixel('nav-2', 'page-child')).toBe(false)
1516
})
1617

17-
it('should_count_revisits_with_a_fresh_navigation_key', () => {
18-
expect(shouldFirePixel('nav-3', 'page-a')).toBe(true)
19-
// Back/forward navigation produces a new location.key — revisits count.
20-
expect(shouldFirePixel('nav-4', 'page-a')).toBe(true)
18+
it('should_count_back_forward_revisits_that_restore_the_original_key', () => {
19+
// location.key is stable per history entry: visiting A, pushing to B,
20+
// then pressing Back POPs to A's ORIGINAL key. The key transition (not
21+
// key novelty) is what marks a new navigation.
22+
expect(shouldFirePixel('key-a', 'page-a')).toBe(true)
23+
expect(shouldFirePixel('key-b', 'page-b')).toBe(true)
24+
expect(shouldFirePixel('key-a', 'page-a')).toBe(true)
25+
// Within the restored entry, duplicates still dedupe.
26+
expect(shouldFirePixel('key-a', 'page-a')).toBe(false)
2127
})
2228
})
Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,27 @@
1-
// One pixel per page per navigation entry. Nested compositions mount
2-
// multiple Weaverse instances on one URL (and React StrictMode re-runs
3-
// mount effects); both would otherwise double-count the same impression.
4-
// Back/forward navigation gets a fresh `location.key`, so revisits count.
5-
let firedPixels = new Set<string>()
1+
// One pixel per page per navigation. Nested compositions mount multiple
2+
// Weaverse instances on one URL (and React StrictMode re-runs mount
3+
// effects); both would otherwise double-count the same impression.
4+
//
5+
// `location.key` is stable per history entry, so a back/forward POP
6+
// restores the entry's ORIGINAL key — a persistent "seen keys" set would
7+
// suppress those revisits. Instead, remember only the current key and
8+
// reset on every key change: any navigation (push, replace, or pop)
9+
// transitions the key and counts again, while re-mounts and co-located
10+
// duplicates within one navigation dedupe.
11+
let currentNavigationKey: string | null = null
12+
let firedPageIds = new Set<string>()
613

714
export function shouldFirePixel(
815
navigationKey: string,
916
pageId: string
1017
): boolean {
11-
let key = `${navigationKey}:${pageId}`
12-
if (firedPixels.has(key)) {
18+
if (navigationKey !== currentNavigationKey) {
19+
currentNavigationKey = navigationKey
20+
firedPageIds.clear()
21+
}
22+
if (firedPageIds.has(pageId)) {
1323
return false
1424
}
15-
firedPixels.add(key)
25+
firedPageIds.add(pageId)
1626
return true
1727
}

0 commit comments

Comments
 (0)