Skip to content

Preserve distinct manual pageviews in v12 - #108

Merged
adriaandotcom merged 5 commits into
v12from
fix/v12-manual-pageview-paths
Oct 15, 2026
Merged

adriaandotcom merged 5 commits into
v12from
fix/v12-manual-pageview-paths

Conversation

@simple-analytics-ai

@simple-analytics-ai simple-analytics-ai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Summary

Closes #107

Manual page views such as /product?product-id=1 and /product?product-id=2 were deduplicated as the same path, so only the first one was counted. src/default.js now builds the duplicate key from the cleaned path plus the manual query (lastSendPath = path + querySearch) instead of the cleaned path alone, so both are counted while an identical consecutive call stays suppressed.

- duplicate key = cleaned path
+ duplicate key = cleaned path + manual query

The existing myPathOverwriter behavior is preserved: the query is removed before getPath() runs, so the callback still receives { path: "/product" }. Ignore-pages matching, hash routes, numeric manual paths and numeric callback return values are unchanged. The reported path never contains the query; the manual query is only used internally as part of the duplicate key, and is still filtered through getQueryParams() before anything is sent.

An earlier proposal to pass queries into the callback is superseded and needs no documentation change. Browser-query attribution is handled separately in #109.

v12 at 38248a0 was merged into this branch. The committed dist/ has not been regenerated since that merge, so it is currently older than the base branch and drops the inline-events feature from #106; it needs a pnpm run build before merge.

Changes:

  • Preserve distinct manual pageviews in v12 - Requested in the compatibility review: fix query-only manual pageview loss and preserve full path-overwriter input. Keep the filtered outgoing path and review campaign attribution...
  • Preserve ignore-pages and hash routes for manual query paths - Address automated review of item 1: normalize the manual query after the path-overwriter callback and before ignore-pages/hash handling, while keeping its query in the duplicate k...
  • Keep non-string manual paths compatible during query normalization - Address review of item 1: only split string path values, preserving numeric manual paths and numeric path-overwriter results without introducing an exception into the customer pag...
  • Preserve pathname-only input for path overwriters - Requested during review: keep the existing path-overwriter behavior without query strings, while retaining distinct manual-query pageviews. Restore query splitting before getPath...
  • Merge v12 into manual-pageview compatibility fix - Requested: merge the current v12 branch into PR Preserve distinct manual pageviews in v12 #108 and defer dist compilation until the end. Preserve pathname-only path-overwriter input and query-aware duplicate detection alo...

Security implications

Has security impact - described as: manual page views that differ only in their query string are no longer collapsed into one, so more page view records are collected for the same path. No additional personal data is sent, because the query string is still stripped from the reported path and query parameters remain filtered by the existing allow-params and UTM rules.

Testing

  • npx mocha "test/unit/*.test.js" --exit against the committed dist/ at 1787a45: 15 failing, 23 passing — every test in test/unit/inline-events.test.js fails because the committed bundles predate the Support inline events in the main v12 script #106 merge.
  • node compile.js then npx mocha "test/unit/*.test.js" --exit: 38 passing, covering inline events, distinct-query manual page views, identical repeated calls, ignore-pages, hash routes, numeric paths/results, and pathname-only callback input. The working tree was restored afterwards, so dist/ on this branch is still stale.
  • Run on Node.js 22.23.3 with npm install (the declared Node 24 / pnpm 11 toolchain was not available in the review environment).
  • BrowserStack left to CI.

Checklist

  • Linked to an issue
  • Tested
  • Asked for a review

Requested in the compatibility review: fix query-only manual pageview loss and preserve full path-overwriter input. Keep the filtered outgoing path and review campaign attribution separately.

Refs #107
Comment thread src/default.js Outdated
Comment thread src/default.js Outdated
@github-actions

Copy link
Copy Markdown

Documentation update

_docs/20_script-settings/03_overwrite-path.md

Location: After the paragraph starting with "You can specify the path callback function via data-path-overwriter", before the closing "If you want to omit those page views completely" line

Copy and paste this into the documentation repository:

When you trigger a page view manually and pass a path that contains a query string, for example `sa.pageview("/product?product-id=1")`, the `path` key given to your function contains that query string as well (`/product?product-id=1`). Take this into account when you compare the path, for instance by splitting on `?` first:

```html
<script>
  function myPathOverwriter({ path }) {
    var pathname = path.split("?")[0];
    if (pathname.startsWith("/profiles/")) return "/profiles/***";
    return path;
  }
</script>

For automatically collected page views the path key never contains a query string.

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Claude review checkpoint

Reviewed commit 1787a4599dcfee98edb13ff0c38759c5f55062e3. This is used to keep later automated reviews focused on changes Claude has not checked yet.

@github-actions github-actions Bot added the change: needs review Changes affecting security, data protection, or system stability. label Oct 15, 2026
@github-actions

Copy link
Copy Markdown

SDLC label: change: needs review because the diff passes the query string into getPath(), so shouldIgnore() stops matching configured data-ignore-pages entries and manual page views the customer opted out of are collected

Address automated review of item 1: normalize the manual query after the path-overwriter callback and before ignore-pages/hash handling, while keeping its query in the duplicate key.

Refs #107
Comment thread src/default.js Outdated
Address review of item 1: only split string path values, preserving numeric manual paths and numeric path-overwriter results without introducing an exception into the customer page.

Refs #107
@simple-analytics-ai

simple-analytics-ai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Author

In response to #108 (comment), superseding #108 (comment):

Confirmed in 15e99ef: pageview splits the manual query before calling getPath, so the path-overwriter callback receives { path: "/product" }. I removed the obsolete query-bearing callback patch from the pending docs bundle. No documentation change is needed for #108.

The separate query-attribution clarification requested in #109 is tracked there and remains unpublished pending the existing request for approval of a supporting docs PR.

Requested during review: keep the existing path-overwriter behavior without query strings, while retaining distinct manual-query pageviews. Restore query splitting before getPath and update the callback and duplicate checks.
simple-analytics-ai Bot pushed a commit that referenced this pull request Oct 15, 2026
Carry the requested callback compatibility correction from PR #108 into its dependent attribution PR. Remove queries before getPath, preserve distinct-query duplicate detection, and regenerate the bundles. All 21 unit tests and both builds pass.
@github-actions

Copy link
Copy Markdown

The earlier documentation update comment and the docs patch prepared from it are now incorrect: 15e99ef moved the query stripping back above getPath(), so myPathOverwriter receives { path: "/product" }, not { path: "/product?product-id=1" }. Publishing that patch would document behavior this PR does not have — drop it and leave _docs/20_script-settings/03_overwrite-path.md unchanged.

Requested: merge the current v12 branch into PR #108 and defer dist compilation until the end. Preserve pathname-only path-overwriter input and query-aware duplicate detection alongside v12 inline events. The combined source passes all 38 unit tests and formatting checks in a temporary build; repository dist files are unchanged.
Comment thread dist/latest/latest.dev.js
@adriaandotcom
adriaandotcom merged commit ee45d52 into v12 Oct 15, 2026
2 checks passed
@adriaandotcom
adriaandotcom deleted the fix/v12-manual-pageview-paths branch October 15, 2026 14:32
simple-analytics-ai Bot pushed a commit that referenced this pull request Oct 15, 2026
Requested: rebuild the distributions after PR #108 and verify that manual queries override browser parameters, absent manual queries fall back to location.search, and existing filtering remains effective. Include the newly merged phone-click fix from #111 and preserve current inline events.

Regenerate only the 29 affected bundles and source maps. All 39 unit tests and 24 additional payload checks pass on the pinned toolchain.

Refs #107
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change: needs review Changes affecting security, data protection, or system stability.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant