refactor: streamline BetterBugs script loading in index.html#41639
refactor: streamline BetterBugs script loading in index.html#41639sebastianiv21 wants to merge 1 commit intoreleasefrom
Conversation
Replaced the previous script loading method for BetterBugs with a more efficient inline function that checks for recording status before appending the necessary scripts. This change enhances performance by conditionally loading scripts only when required, improving the overall user experience.
|
/build-deploy-preview skip-tests=true |
WalkthroughThe BetterBugs script loader in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/23329077839. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/client/public/index.html`:
- Line 84: The dynamic loader currently uses o(n) to create async scripts and
document.head.prepend, which reverses required order and sets async=true; modify
the loader so that logs-capture.js is inserted before recorder.js and not marked
async: change o(n) to append scripts via document.head.appendChild (or create
the first script for "https://cdn.betterbugs.io/scripts/latest/logs-capture.js"
and attach an onload handler that then creates and appends the recorder script),
and remove r.async = true so scripts load and execute in sequence; update code
paths that call n() and o() accordingly to ensure "logs-capture.js" always loads
and finishes before "recorder.js".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 677a22dd-0047-4aa0-80d0-2ff175483cf3
📒 Files selected for processing (1)
app/client/public/index.html
| s2.async = true; | ||
| document.head.appendChild(s2); | ||
| })(); | ||
| !function(){function n(){var r=new URLSearchParams(window.location.search);if(r.has("betterbugs-recording"))return!0;try{var e=localStorage.getItem("recordingStatuses");if(e){var t=JSON.parse(e);if(Array.isArray(t)&&t.some(function(n){return n&&!0===n.currentlyRecording}))return!0}}catch(n){}return!1}function o(n){var r=document.createElement("script");r.src=n,r.async=!0,document.head.prepend(r)}n()&&(o("https://cdn.betterbugs.io/scripts/latest/logs-capture.js"),o("https://cdn.betterbugs.io/scripts/latest/recorder.js"))}(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does BetterBugs recording-link integration document whether logs-capture.jsandrecorder.jscan be injected in any order, and whether dynamically loading both scripts withasync=true is supported?
💡 Result:
No, the BetterBugs recording-link integration documentation does not explicitly state whether logs-capture.js and recorder.js can be injected in any order, nor does it document support for dynamically loading both scripts with async=true. The official documentation (primarily at https://docs.betterbugs.io/dashboard/workspace-settings/recording-links) instructs to add both scripts sequentially to the HTML tag without async attributes: <script src="https://cdn.betterbugs.io/scripts/latest/logs-capture.js"></script> <script src="https://cdn.betterbugs.io/scripts/latest/recorder.js"></script> This implies logs-capture.js loads first, followed by recorder.js. No mention is made of order flexibility, async/defer attributes, or dynamic injection (e.g., via JavaScript document.createElement('script') with async=true). Examples consistently show this static, blocking order in . No troubleshooting, configuration options, or GitHub source code addresses dependencies or async loading compatibility.
Citations:
- 1: https://docs.betterbugs.io/product-features/recording-links/recording-links-with-developer-logs
- 2: https://docs.betterbugs.io/dashboard/workspace-settings/recording-links
- 3: https://docs.betterbugs.io/product-features/recording-links/using-recording-links
- 4: https://docs.betterbugs.io/product-features/recording-links
- 5: https://docs.betterbugs.io/configuration-options
- 6: https://github.com/betterbugs/web-sdk
- 7: https://docs.betterbugs.io/
- 8: https://docs.betterbugs.io/developer-guide/betterbugs-web-sdk/installation/start-widget-with-cdn-script
- 9: https://www.mylizzy.com
- 10: https://docs.betterbugs.io/overview
Fix the BetterBugs script loading order and async strategy.
BetterBugs documentation explicitly requires logs-capture.js to load before recorder.js, as synchronous blocking scripts. Using prepend() reverses this critical order, and async=true is not documented as supported for dynamic injection. Use appendChild() instead of prepend(), or chain the second script to the first's onload handler to maintain proper sequencing and loading behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/client/public/index.html` at line 84, The dynamic loader currently uses
o(n) to create async scripts and document.head.prepend, which reverses required
order and sets async=true; modify the loader so that logs-capture.js is inserted
before recorder.js and not marked async: change o(n) to append scripts via
document.head.appendChild (or create the first script for
"https://cdn.betterbugs.io/scripts/latest/logs-capture.js" and attach an onload
handler that then creates and appends the recorder script), and remove r.async =
true so scripts load and execute in sequence; update code paths that call n()
and o() accordingly to ensure "logs-capture.js" always loads and finishes before
"recorder.js".
|
Deploy-Preview-URL: https://ce-41639.dp.appsmith.com |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
1 participant
Description
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Replaced the previous script loading method for BetterBugs with a more efficient inline function that checks for recording status before appending the necessary scripts. This change enhances performance by conditionally loading scripts only when required, improving the overall user experience.
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/23329064176
Commit: 5d8b25b
Cypress dashboard.
Tags:
@tag.SanitySpec:
Fri, 20 Mar 2026 05:08:12 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Bug Fixes