Gate SimpleAnalytics injection behind VITE_APP_DISABLE_TRACKING - #2
Gate SimpleAnalytics injection behind VITE_APP_DISABLE_TRACKING#2dimafa wants to merge 2 commits into
Conversation
The third-party SimpleAnalytics script (scripts.simpleanalyticscdn.com) was injected on every PROD build, ignoring VITE_APP_DISABLE_TRACKING. Self-hosters who set that flag (as the excalidraw-full deployment does) still shipped a third-party tracker that beacons page paths. Gate the injection on the flag, matching the existing template pattern used for VITE_APP_DEV_DISABLE_LIVE_RELOAD. Default behavior is unchanged when the flag is unset. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates the analytics script inclusion in excalidraw-app/index.html to be skipped when the VITE_APP_DISABLE_TRACKING environment variable is set to 'yes'. The reviewer suggests enhancing this check to also support 'true' or boolean true values, making the configuration more robust and user-friendly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| <% if (typeof PROD != 'undefined' && PROD == true && (typeof VITE_APP_DISABLE_TRACKING == 'undefined' || VITE_APP_DISABLE_TRACKING != 'yes')) { %> | ||
| <!-- Analytics (skipped when VITE_APP_DISABLE_TRACKING=yes) --> |
There was a problem hiding this comment.
To make the tracking disablement more robust, we should also handle cases where VITE_APP_DISABLE_TRACKING is set to "true" or boolean true. Users frequently use these values for boolean-like environment variables, and only checking for "yes" might lead to unexpected tracking if they configure it as true.
<% if (typeof PROD != 'undefined' && PROD == true && (typeof VITE_APP_DISABLE_TRACKING == 'undefined' || (VITE_APP_DISABLE_TRACKING != 'yes' && VITE_APP_DISABLE_TRACKING != 'true' && VITE_APP_DISABLE_TRACKING !== true))) { %>
<!-- Analytics (skipped when VITE_APP_DISABLE_TRACKING is enabled) -->…TRACKING Users often set boolean-like env vars to true; accept 'yes', 'true', or boolean true so tracking is reliably disabled regardless of the form used. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Good point — applied. Now treats |
What
The third-party SimpleAnalytics script (
scripts.simpleanalyticscdn.com/latest.js) is injected inexcalidraw-app/index.htmlon every PROD build — the injection is gated only byPROD, not byVITE_APP_DISABLE_TRACKING. As a result, self-hosted deployments that setVITE_APP_DISABLE_TRACKING=yes(e.g. theexcalidraw-fullDocker build) still ship a third-party tracker that beacons page paths on every load.Change
Gate the analytics injection on the existing flag:
<% if (typeof PROD != 'undefined' && PROD == true && (typeof VITE_APP_DISABLE_TRACKING == 'undefined' || VITE_APP_DISABLE_TRACKING != 'yes')) { %>This follows the same template pattern already used for
VITE_APP_DEV_DISABLE_LIVE_RELOAD(index.html L145). When the flag is unset, behavior is unchanged; whenVITE_APP_DISABLE_TRACKING=yes, the tracker is not injected.Why
Privacy: a self-hosted instance shouldn't phone home to a third party, and the project already exposes a flag whose name implies exactly this. One-line, behavior-preserving by default.
Notes
VITE_APP_ENABLE_TRACKINGused by thebuild:appscript; this PR intentionally honors theDISABLE_TRACKINGflag the self-host path sets, but happy to also key offENABLE_TRACKINGif you'd prefer a single source of truth.