-
-
Notifications
You must be signed in to change notification settings - Fork 441
feat: Remove script element immediately in injectScript #1761
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Remove script element immediately in injectScript #1761
Conversation
✅ Deploy Preview for creative-fairy-df92c4 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, OK nice. If it works, it works! I just noticed your series of PRs... Might re-open the one I closed.
Everything seems to work: the script is executed; onload, onerror handlers do the right thing; document.currentScript invoked by the script returns the detached script element.
300b769 to
41833c6
Compare
Unless I misunderstand which PR you meant by the one you closed, it was one of the three PRs for the same issue but not the one posted by me. |
@wxt-dev/analytics
@wxt-dev/auto-icons
@wxt-dev/browser
@wxt-dev/i18n
@wxt-dev/module-react
@wxt-dev/module-solid
@wxt-dev/module-svelte
@wxt-dev/module-vue
@wxt-dev/runner
@wxt-dev/storage
@wxt-dev/unocss
@wxt-dev/webextension-polyfill
wxt
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1761 +/- ##
==========================================
- Coverage 76.62% 76.42% -0.20%
==========================================
Files 113 113
Lines 3025 3025
Branches 686 686
==========================================
- Hits 2318 2312 -6
- Misses 624 629 +5
- Partials 83 84 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks for helping make WXT better! |
Note
The patch series in which each PR builds on top of the previous one: #1761 (you are here), #1762, #1763, #1838, #1765.
In case some of the changes are rejected, I will rebase the subsequent PRs on top of main.
Overview
I am trying to minimize any chance of my extension leaving something in the DOM or the window object etc.
A script which fails to load would be left in the DOM. Rather than adding an
onerrorhandler, it seems that the script element can just be removed immediately after being added.Everything seems to work: the script is executed;
onload,onerrorhandlers do the right thing;document.currentScriptinvoked by the script returns the detached script element.Manual Testing
Just using
injectScriptnormally should suffice. I am working on an extension which usesinjectScript. It seems to work fine with this change.Related Issue
N/A