Skip to content

fix analytis script URL to avoid one more request on our CDN#1520

Merged
guitavano merged 2 commits intomainfrom
tavano/fix-analytics-url
Feb 5, 2026
Merged

fix analytis script URL to avoid one more request on our CDN#1520
guitavano merged 2 commits intomainfrom
tavano/fix-analytics-url

Conversation

@guitavano
Copy link
Contributor

@guitavano guitavano commented Feb 5, 2026

Summary by cubic

Switched the analytics script to load directly from the static URL by default, removing an extra CDN request and improving load time. Added an env flag to optionally keep using the site-domain proxy.

  • Migration
    • Set USE_SITE_DOMAIN_FOR_ANALYTICS_SCRIPT=true to keep proxying via /live/invoke.
    • No action needed to use the direct script; default is false.

Written for commit b75fe60. Summary will update on new commits.

Summary by CodeRabbit

  • Infrastructure
    • Added environment-based configuration for analytics script routing to support flexible data collection paths.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2026

Tagging Options

Should a new tag be published when this PR is merged?

  • 👍 for Patch 0.133.29 update
  • 🎉 for Minor 0.134.0 update
  • 🚀 for Major 1.0.0 update

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 5, 2026

📝 Walkthrough

Walkthrough

This PR adds conditional routing for analytics scripts through a new environment-controlled flag. A boolean constant USE_SITE_DOMAIN_FOR_ANALYTICS_SCRIPT is introduced and exported from OneDollarStats.tsx, then imported in the OneDollarScript loader to conditionally route the analytics script URL through a loader path or use it directly.

Changes

Cohort / File(s) Summary
Analytics Script Routing
analytics/loaders/OneDollarScript.ts, website/components/OneDollarStats.tsx
Introduces environment-driven flag USE_SITE_DOMAIN_FOR_ANALYTICS_SCRIPT to conditionally route analytics script through /live/invoke/website/loaders/analyticsScript.ts endpoint when enabled, or use static script URL directly when disabled.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • tlgimenes

Poem

🐰 Analytics scripts hop with grace,
Through conditional pathways they race,
One flag to guide their flow,
Direct or detoured—either way to go! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: optimizing the analytics script URL to reduce CDN requests.
Description check ✅ Passed The description covers the main change and migration steps, but is missing required template sections like Issue Link and Loom Video.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch tavano/fix-analytics-url

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
analytics/loaders/OneDollarScript.ts (1)

79-88: ⚠️ Potential issue | 🟠 Major

Encode staticScriptUrl before embedding it in the query string.
Unencoded URLs can break the loader query (e.g., & splits params), causing script load failures.

✅ Suggested fix
-    const trackerScript = `<script
+    const trackerSrc = USE_SITE_DOMAIN_FOR_ANALYTICS_SCRIPT
+      ? `/live/invoke/website/loaders/analyticsScript.ts?url=${encodeURIComponent(staticScriptUrl)}`
+      : staticScriptUrl;
+    const trackerScript = `<script
         id="tracker"
         data-autocollect="false"
         data-hash-routing="true"
         data-url="${collector}"
-        src="${
-      USE_SITE_DOMAIN_FOR_ANALYTICS_SCRIPT
-        ? `/live/invoke/website/loaders/analyticsScript.ts?url=${staticScriptUrl}`
-        : staticScriptUrl
-    }"
+        src="${trackerSrc}"
       ></script>`;
🤖 Fix all issues with AI agents
In `@website/components/OneDollarStats.tsx`:
- Around line 107-109: When building the proxied analytics URL in the
OneDollarStats component, the query value uses raw staticScript which can break
if it contains characters like & or #; update the src construction that checks
USE_SITE_DOMAIN_FOR_ANALYTICS_SCRIPT to pass encodeURIComponent(staticScript)
(referencing the OneDollarStats component, the staticScript variable and the
USE_SITE_DOMAIN_FOR_ANALYTICS_SCRIPT flag) so the script URL is properly
URL-encoded when proxied through /live/invoke.
🧹 Nitpick comments (1)
analytics/loaders/OneDollarScript.ts (1)

3-5: Consider moving the env-derived flag to a shared config module.
Importing a TSX UI module from a loader couples server logic to UI code and forces evaluation of the component module. Extract the flag (and related constants, if useful) to a dedicated config file consumed by both sides.

Comment on lines +107 to +109
src={USE_SITE_DOMAIN_FOR_ANALYTICS_SCRIPT
? `/live/invoke/website/loaders/analyticsScript.ts?url=${staticScript}`
: staticScript}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

URL-encode staticScript when proxying through /live/invoke.
Unencoded query values can break when the URL includes & or #, leading to failed script loads.

✅ Suggested fix
-        src={USE_SITE_DOMAIN_FOR_ANALYTICS_SCRIPT
-          ? `/live/invoke/website/loaders/analyticsScript.ts?url=${staticScript}`
-          : staticScript}
+        src={USE_SITE_DOMAIN_FOR_ANALYTICS_SCRIPT
+          ? `/live/invoke/website/loaders/analyticsScript.ts?url=${encodeURIComponent(staticScript)}`
+          : staticScript}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
src={USE_SITE_DOMAIN_FOR_ANALYTICS_SCRIPT
? `/live/invoke/website/loaders/analyticsScript.ts?url=${staticScript}`
: staticScript}
src={USE_SITE_DOMAIN_FOR_ANALYTICS_SCRIPT
? `/live/invoke/website/loaders/analyticsScript.ts?url=${encodeURIComponent(staticScript)}`
: staticScript}
🤖 Prompt for AI Agents
In `@website/components/OneDollarStats.tsx` around lines 107 - 109, When building
the proxied analytics URL in the OneDollarStats component, the query value uses
raw staticScript which can break if it contains characters like & or #; update
the src construction that checks USE_SITE_DOMAIN_FOR_ANALYTICS_SCRIPT to pass
encodeURIComponent(staticScript) (referencing the OneDollarStats component, the
staticScript variable and the USE_SITE_DOMAIN_FOR_ANALYTICS_SCRIPT flag) so the
script URL is properly URL-encoded when proxied through /live/invoke.

@guitavano guitavano merged commit 783d179 into main Feb 5, 2026
7 checks passed
@guitavano guitavano deleted the tavano/fix-analytics-url branch February 5, 2026 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants