feat: skip attaching the launcher when its embedded and update tests #53
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request optimizes the Rokt launcher initialization by detecting when the launcher.js is already embedded on the page and skipping redundant script tag creation. When window.Rokt.createLauncher exists, the code now calls attachLauncher() directly instead of fetching launcher.js via a new script tag, unless the launcher has already been initialized.
Changes:
- Modified initialization logic to check for embedded launcher before creating script tags
- Removed misleading console warning when Rokt is already present
- Added test coverage for embedded launcher scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Rokt-Kit.js | Updated initForwarder to check if launcher is embedded and call attachLauncher directly, eliminating redundant script loading |
| test/src/tests.js | Updated existing test to force script loading path and added new tests to verify embedded launcher behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/Rokt-Kit.js
Outdated
| return ( | ||
| window.Rokt && | ||
| typeof window.Rokt.createLauncher === 'function' && | ||
| window.Rokt.currentLauncher === undefined |
There was a problem hiding this comment.
what's the reason for checking window.Rokt.currentLauncher === undefined?
Is there a scenario where we would ever have window.Rokt, window.Rokt.createLauncher and already have a currentLauncher?
There was a problem hiding this comment.
The check already existed in the script.onload callback (line 148), so I kept it for consistency and as a defensive measure. Rather than having different logic in two places, I DRYed it up into the isLauncherEmbedded() helper so both paths use the same validation
Summary
window.Rokt.createLauncheralready exists, directly callattachLauncher() instead of fetching launcher.js via a new script tagTesting Plan