[PULSE-51] Visitor ID storage: optional localStorage for cross-tab unique visitors #21
@@ -89,6 +89,9 @@ export default function ScriptSetupBlock({
|
||||
<code className="text-xs text-neutral-900 dark:text-white break-all font-mono block pr-10">
|
||||
{`<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="local"</code> for cross-tab unique visitors; <code className="rounded px-1 bg-neutral-200 dark:bg-neutral-700">data-storage-ttl="24"</code> (hours) to expire the ID.
|
||||
</p>
|
||||
|
|
||||
<button
|
||||
type="button"
|
||||
onClick={copyScript}
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
/**
|
||||
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 AIDefault 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>
Issue: Default was documented as sessionStorage while the PR said “keep sessionStorage as default.” 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
|
||||
* Lightweight, no cookies, GDPR compliant.
|
||||
* Default: ephemeral session ID (sessionStorage, per-tab). Optional: data-storage="local"
|
||||
* and data-storage-ttl (hours) for a persistent cross-tab visitor ID with optional expiry.
|
||||
*/
|
||||
|
||||
(function() {
|
||||
@@ -19,6 +21,11 @@
|
||||
|
||||
|
Prompt To Fix With AI`parseFloat` with two arguments (second argument is radix) is incorrect - `parseFloat` only accepts one argument. The radix parameter is for `parseInt`, not `parseFloat`.
```suggestion
const ttlHours = storageMode === 'local' ? parseFloat(script.getAttribute('data-storage-ttl') || '24') : 0;
```
<details><summary>Prompt To Fix With AI</summary>
`````markdown
This is a comment left during a code review.
Path: public/script.js
Line: 27:27
Comment:
`parseFloat` with two arguments (second argument is radix) is incorrect - `parseFloat` only accepts one argument. The radix parameter is for `parseInt`, not `parseFloat`.
```suggestion
const ttlHours = storageMode === 'local' ? parseFloat(script.getAttribute('data-storage-ttl') || '24') : 0;
```
How can I resolve this? If you propose a fix, please make it concise.
`````
</details>
Issue: parseFloat was called with two arguments; only parseInt accepts a radix. Issue: parseFloat was called with two arguments; only parseInt accepts a radix.
Fix: Removed the second argument so the call is parseFloat(script.getAttribute('data-storage-ttl') || '24').
Why: Correct use of the API.
|
||||
const domain = script.getAttribute('data-domain');
|
||||
const apiUrl = script.getAttribute('data-api') || 'https://pulse-api.ciphera.net';
|
||||
// * Visitor ID storage: "session" (default, ephemeral per-tab) or "local" (persistent, cross-tab)
|
||||
const storageMode = (script.getAttribute('data-storage') || 'session').toLowerCase() === 'local' ? 'local' : 'session';
|
||||
// * When storage is "local", optional TTL in hours; after TTL the ID is regenerated (e.g. 24 = one day)
|
||||
const ttlHours = storageMode === 'local' ? parseFloat(script.getAttribute('data-storage-ttl') || '24', 10) : 0;
|
||||
const ttlMs = ttlHours > 0 ? ttlHours * 60 * 60 * 1000 : 0;
|
||||
|
||||
// * Performance Monitoring (Core Web Vitals) State
|
||||
let currentEventId = null;
|
||||
@@ -102,25 +109,51 @@
|
||||
}
|
||||
|
Comment states "Default: ephemeral (sessionStorage, per-tab)" but the actual default in line 25 is Prompt To Fix With AIComment states "Default: ephemeral (sessionStorage, per-tab)" but the actual default in line 25 is `'local'` (localStorage). This is a contradiction - either the comment or the code is incorrect.
<details><summary>Prompt To Fix With AI</summary>
`````markdown
This is a comment left during a code review.
Path: public/script.js
Line: 119:120
Comment:
Comment states "Default: ephemeral (sessionStorage, per-tab)" but the actual default in line 25 is `'local'` (localStorage). This is a contradiction - either the comment or the code is incorrect.
How can I resolve this? If you propose a fix, please make it concise.
`````
</details>
Issue: Comment said “Default: ephemeral (sessionStorage, per-tab)” but the code default was 'local'. Issue: Comment said “Default: ephemeral (sessionStorage, per-tab)” but the code default was 'local'.
Fix: Comment updated to: “Default: persistent (localStorage, cross-tab), optional TTL in hours. With data-storage=\"session\": ephemeral (sessionStorage, per-tab).” Default in code remains 'local'.
Why: Comment and code now both describe the cross-tab default.
regenerating ID after all re-reads means two tabs opening simultaneously will create different IDs even when they shouldn't - the second Prompt To Fix With AIregenerating ID after all re-reads means two tabs opening simultaneously will create different IDs even when they shouldn't - the second `generateId()` call creates a new ID rather than reusing the one from line 146, causing both tabs to write their own unique IDs
<details><summary>Prompt To Fix With AI</summary>
`````markdown
This is a comment left during a code review.
Path: public/script.js
Line: 176:176
Comment:
regenerating ID after all re-reads means two tabs opening simultaneously will create different IDs even when they shouldn't - the second `generateId()` call creates a new ID rather than reusing the one from line 146, causing both tabs to write their own unique IDs
How can I resolve this? If you propose a fix, please make it concise.
`````
</details>
Issue: Regenerating the ID before the write meant two tabs could each get a new ID and both write, so we could end up with two different IDs and duplicate counts. Issue: Regenerating the ID before the write meant two tabs could each get a new ID and both write, so we could end up with two different IDs and duplicate counts.
Fix: Removed the second generateId(); we now use the ID from line 146 when writing.
Why: Avoids introducing a second ID in the write path so we don’t make the simultaneous-open case worse.
|
||||
});
|
||||
|
||||
// * Memory cache for session ID (fallback if sessionStorage is unavailable)
|
||||
// * Memory cache for session ID (fallback if storage is unavailable)
|
||||
let cachedSessionId = null;
|
||||
|
||||
// * Generate ephemeral session ID (not persistent)
|
||||
function generateId() {
|
||||
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.
|
||||
function getSessionId() {
|
||||
if (cachedSessionId) {
|
||||
return cachedSessionId;
|
||||
}
|
||||
|
||||
// * Use a static key for session storage to ensure consistency across pages
|
||||
const key = 'ciphera_session_id';
|
||||
// * Legacy key support for migration (strip whitespace just in case)
|
||||
const legacyKey = 'plausible_session_' + (domain ? domain.trim() : '');
|
||||
|
||||
try {
|
||||
// * Try to get existing session ID
|
||||
cachedSessionId = sessionStorage.getItem(key);
|
||||
if (storageMode === 'local') {
|
||||
try {
|
||||
const raw = localStorage.getItem(key);
|
||||
if (raw) {
|
||||
try {
|
||||
const parsed = JSON.parse(raw);
|
||||
if (parsed && typeof parsed.id === 'string') {
|
||||
const expired = ttlMs > 0 && typeof parsed.created === 'number' && (Date.now() - parsed.created > ttlMs);
|
||||
if (!expired) {
|
||||
cachedSessionId = parsed.id;
|
||||
return cachedSessionId;
|
||||
}
|
||||
}
|
||||
} catch (e) {
|
||||
// * Invalid JSON or old string format: treat as expired and regenerate
|
||||
}
|
||||
}
|
||||
cachedSessionId = generateId();
|
||||
localStorage.setItem(key, JSON.stringify({ id: cachedSessionId, created: Date.now() }));
|
||||
} catch (e) {
|
||||
cachedSessionId = generateId();
|
||||
}
|
||||
return cachedSessionId;
|
||||
}
|
||||
|
||||
// * If not found in new key, try legacy key and migrate
|
||||
// * Default: session storage (ephemeral, per-tab)
|
||||
try {
|
||||
cachedSessionId = sessionStorage.getItem(key);
|
||||
if (!cachedSessionId && legacyKey) {
|
||||
cachedSessionId = sessionStorage.getItem(legacyKey);
|
||||
|
Legacy value causes 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**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>
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. 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.
|
||||
if (cachedSessionId) {
|
||||
@@ -133,7 +166,7 @@
|
||||
}
|
||||
|
||||
if (!cachedSessionId) {
|
||||
cachedSessionId = Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15);
|
||||
cachedSessionId = generateId();
|
||||
try {
|
||||
sessionStorage.setItem(key, cachedSessionId);
|
||||
} catch (e) {
|
||||
|
||||
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 thatdata-storage="session"opts OUT of cross-tab tracking.Prompt To Fix With AI
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.
Missing documentation for
data-storage-ttlattribute. 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
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.