ciphera-net/pulse
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 15 Wiki Activity

[PULSE-51] Visitor ID storage: optional localStorage for cross-tab unique visitors #21

Merged
uz1mani merged 17 commits from staging into main 2026-02-11 15:14:13 +00:00
Conversation 28 Commits 17 Files Changed 3 +74 -10
2 changed files with 74 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 1454d15142 - Show all commits

2
components/sites/ScriptSetupBlock.tsx
View File

@@ -90,7 +90,7 @@ export default function ScriptSetupBlock({
{`<script defer data-domain="${site.domain}" data-api="${API_URL}" src="${APP_URL}/script.js"></script>`}
</code>
<p className="mt-2 text-xs text-neutral-500 dark:text-neutral-400">
Optional: <code className="rounded px-1 bg-neutral-200 dark:bg-neutral-700">data-storage=&quot;session&quot;</code> for per-tab (ephemeral) visitor counting.
Default: cross-tab (localStorage). Optional: <code className="rounded px-1 bg-neutral-200 dark:bg-neutral-700">data-storage=&quot;session&quot;</code> to opt out (per-tab, ephemeral).
</p>
greptile-apps[bot] commented 2026-02-11 13:04:49 +00:00 (Migrated from github.com)
Review
Copy Link

Documentation says data-storage="session" is "Optional" but according to the implementation, this should be the default. If localStorage is now the default, this text should clarify that data-storage="session" opts OUT of cross-tab tracking.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: components/sites/ScriptSetupBlock.tsx
Line: 92:94

Comment:
Documentation says `data-storage="session"` is "Optional" but according to the implementation, this should be the default. If localStorage is now the default, this text should clarify that `data-storage="session"` opts OUT of cross-tab tracking.

How can I resolve this? If you propose a fix, please make it concise.
Documentation says `data-storage="session"` is "Optional" but according to the implementation, this should be the default. If localStorage is now the default, this text should clarify that `data-storage="session"` opts OUT of cross-tab tracking. <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: components/sites/ScriptSetupBlock.tsx Line: 92:94 Comment: Documentation says `data-storage="session"` is "Optional" but according to the implementation, this should be the default. If localStorage is now the default, this text should clarify that `data-storage="session"` opts OUT of cross-tab tracking. How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-11 13:13:00 +00:00 (Migrated from github.com)
Review
Copy Link

Issue: Copy said “Optional: data-storage="session"” without stating that the default is cross-tab and that session opts out.
Fix: Copy updated to: “Default: cross-tab (localStorage). Optional: data-storage="session" to opt out (per-tab, ephemeral).”
Why: Makes it explicit that the default is cross-tab and that data-storage="session" opts out of it.

Issue: Copy said “Optional: data-storage=\"session\"” without stating that the default is cross-tab and that session opts out. Fix: Copy updated to: “Default: cross-tab (localStorage). Optional: data-storage=\"session\" to opt out (per-tab, ephemeral).” Why: Makes it explicit that the default is cross-tab and that data-storage="session" opts out of it.
greptile-apps[bot] commented 2026-02-11 13:35:14 +00:00 (Migrated from github.com)
Review
Copy Link

Missing documentation for data-storage-ttl attribute. The script supports an optional TTL parameter (default 24 hours) but this isn't mentioned here. Consider adding: "Optional: data-storage-ttl=\"48\" to set expiry in hours (default: 24)."

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI 
This is a comment left during a code review.
Path: components/sites/ScriptSetupBlock.tsx
Line: 92:94

Comment:
Missing documentation for `data-storage-ttl` attribute. The script supports an optional TTL parameter (default 24 hours) but this isn't mentioned here. Consider adding: "Optional: `data-storage-ttl=\"48\"` to set expiry in hours (default: 24)."

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.
Missing documentation for `data-storage-ttl` attribute. The script supports an optional TTL parameter (default 24 hours) but this isn't mentioned here. Consider adding: "Optional: `data-storage-ttl=\"48\"` to set expiry in hours (default: 24)." <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: components/sites/ScriptSetupBlock.tsx Line: 92:94 Comment: Missing documentation for `data-storage-ttl` attribute. The script supports an optional TTL parameter (default 24 hours) but this isn't mentioned here. Consider adding: "Optional: `data-storage-ttl=\"48\"` to set expiry in hours (default: 24)." <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-11 14:34:22 +00:00 (Migrated from github.com)
Review
Copy Link

Fix: Already added on line 93: “Optional: data-storage-ttl="48" to set expiry in hours (default: 24).”
Why: So users know they can change the cross-tab visitor ID expiry.
If the comment was from a branch that doesn’t have this yet, merge or cherry-pick the commit that added it, or re-apply that sentence in the same paragraph.

Fix: Already added on line 93: “Optional: data-storage-ttl=\"48\" to set expiry in hours (default: 24).” Why: So users know they can change the cross-tab visitor ID expiry. If the comment was from a branch that doesn’t have this yet, merge or cherry-pick the commit that added it, or re-apply that sentence in the same paragraph.
<button
type="button"

20
public/script.js
View File

@@ -1,7 +1,7 @@
/**
greptile-apps[bot] commented 2026-02-11 13:04:45 +00:00 (Migrated from github.com)
Review
Copy Link

Default behavior changed from sessionStorage to localStorage, contradicting PR description which states "keep sessionStorage as default". Comments say "Default: cross-tab visitor ID (localStorage)" but PR says "Default remains sessionStorage".

Prompt To Fix With AI 
This is a comment left during a code review.
Path: public/script.js
Line: 4:5

Comment:
Default behavior changed from sessionStorage to localStorage, contradicting PR description which states "keep sessionStorage as default". Comments say "Default: cross-tab visitor ID (localStorage)" but PR says "Default remains sessionStorage".

How can I resolve this? If you propose a fix, please make it concise.
Default behavior changed from sessionStorage to localStorage, contradicting PR description which states "keep sessionStorage as default". Comments say "Default: cross-tab visitor ID (localStorage)" but PR says "Default remains sessionStorage". <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: public/script.js Line: 4:5 Comment: Default behavior changed from sessionStorage to localStorage, contradicting PR description which states "keep sessionStorage as default". Comments say "Default: cross-tab visitor ID (localStorage)" but PR says "Default remains sessionStorage". How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-11 13:12:32 +00:00 (Migrated from github.com)
Review
Copy Link

Issue: Default was documented as sessionStorage while the PR said “keep sessionStorage as default.”
Fix: Default is cross-tab (localStorage). File header now says: “Default: cross-tab visitor ID (localStorage), optional data-storage-ttl in hours. Optional: data-storage="session" for per-tab (ephemeral) counting.”
Why: Product decision is to use cross-tab as the default; docs and code are aligned with that.

Issue: Default was documented as sessionStorage while the PR said “keep sessionStorage as default.” Fix: Default is cross-tab (localStorage). File header now says: “Default: cross-tab visitor ID (localStorage), optional data-storage-ttl in hours. Optional: data-storage=\"session\" for per-tab (ephemeral) counting.” Why: Product decision is to use cross-tab as the default; docs and code are aligned with that.
* Pulse - Privacy-First Tracking Script
* Lightweight, no cookies, GDPR compliant.
* Default: cross-tab visitor ID (localStorage, optional data-storage-ttl in hours).
* Default: cross-tab visitor ID (localStorage), optional data-storage-ttl in hours.
* Optional: data-storage="session" for per-tab (ephemeral) counting.
*/
@@ -116,8 +116,8 @@
return Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15);
}
// * Returns session/visitor ID. Default: ephemeral (sessionStorage, per-tab).
// * With data-storage="local": persistent (localStorage, cross-tab), optional TTL in hours.
// * Returns session/visitor ID. Default: persistent (localStorage, cross-tab), optional TTL in hours.
// * With data-storage="session": ephemeral (sessionStorage, per-tab).
function getSessionId() {
if (cachedSessionId) {
return cachedSessionId;
@@ -158,6 +158,20 @@
}
greptile-apps[bot] commented 2026-02-11 14:48:36 +00:00 (Migrated from github.com)
Review
Copy Link

Legacy value causes churn
In the localStorage mode, localStorage.getItem(key) is always JSON.parse’d; if parsing fails (including the “old string format” case mentioned in the comment), the code regenerates a new ID instead of reusing the existing string value. If any existing installs already have a plain-string ID under ciphera_session_id in localStorage, this will force an unexpected visitor-id reset on upgrade. A small migration path (e.g., if raw is a non-empty string and not JSON, treat it as the ID and wrap it into the JSON format with created) would avoid that churn.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI 
This is a comment left during a code review.
Path: public/script.js
Line: 129:132

Comment:
**Legacy value causes churn**
In the `localStorage` mode, `localStorage.getItem(key)` is always `JSON.parse`’d; if parsing fails (including the “old string format” case mentioned in the comment), the code regenerates a new ID instead of reusing the existing string value. If any existing installs already have a plain-string ID under `ciphera_session_id` in `localStorage`, this will force an unexpected visitor-id reset on upgrade. A small migration path (e.g., if `raw` is a non-empty string and not JSON, treat it as the ID and wrap it into the JSON format with `created`) would avoid that churn.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.
**Legacy value causes churn** In the `localStorage` mode, `localStorage.getItem(key)` is always `JSON.parse`’d; if parsing fails (including the “old string format” case mentioned in the comment), the code regenerates a new ID instead of reusing the existing string value. If any existing installs already have a plain-string ID under `ciphera_session_id` in `localStorage`, this will force an unexpected visitor-id reset on upgrade. A small migration path (e.g., if `raw` is a non-empty string and not JSON, treat it as the ID and wrap it into the JSON format with `created`) would avoid that churn. <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: public/script.js Line: 129:132 Comment: **Legacy value causes churn** In the `localStorage` mode, `localStorage.getItem(key)` is always `JSON.parse`’d; if parsing fails (including the “old string format” case mentioned in the comment), the code regenerates a new ID instead of reusing the existing string value. If any existing installs already have a plain-string ID under `ciphera_session_id` in `localStorage`, this will force an unexpected visitor-id reset on upgrade. A small migration path (e.g., if `raw` is a non-empty string and not JSON, treat it as the ID and wrap it into the JSON format with `created`) would avoid that churn. <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-11 14:52:23 +00:00 (Migrated from github.com)
Review
Copy Link

Issue: In localStorage mode, a plain-string value in ciphera_session_id (e.g. from an older script) was treated as invalid and caused a new ID to be generated on upgrade.
Fix: If JSON.parse(raw) throws and raw is a non-empty string, we treat it as a legacy ID: set cachedSessionId = raw.trim(), write { id: cachedSessionId, created: Date.now() } to localStorage (first read only), and return. The same legacy handling is used in the two re-read paths (use the string as ID and return; no write there to avoid extra writes).
Why: Existing plain-string IDs are kept across upgrades and migrated to the new JSON shape in one step.

Issue: In localStorage mode, a plain-string value in ciphera_session_id (e.g. from an older script) was treated as invalid and caused a new ID to be generated on upgrade. Fix: If JSON.parse(raw) throws and raw is a non-empty string, we treat it as a legacy ID: set cachedSessionId = raw.trim(), write { id: cachedSessionId, created: Date.now() } to localStorage (first read only), and return. The same legacy handling is used in the two re-read paths (use the string as ID and return; no write there to avoid extra writes). Why: Existing plain-string IDs are kept across upgrades and migrated to the new JSON shape in one step.
} catch (e2) {}
}
// * Final re-read immediately before write to avoid overwriting a fresher ID from another tab
var rawBeforeWrite = localStorage.getItem(key);
if (rawBeforeWrite) {
try {
var parsedBefore = JSON.parse(rawBeforeWrite);
if (parsedBefore && typeof parsedBefore.id === 'string') {
var expBefore = ttlMs > 0 && typeof parsedBefore.created === 'number' && (Date.now() - parsedBefore.created > ttlMs);
if (!expBefore) {
cachedSessionId = parsedBefore.id;
return cachedSessionId;
}
}
} catch (e3) {}
}
localStorage.setItem(key, JSON.stringify({ id: cachedSessionId, created: Date.now() }));
} catch (e) {
cachedSessionId = generateId();