-
Notifications
You must be signed in to change notification settings - Fork 162
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
change script injection approach to use a temporary <script
tag
#1164
Conversation
document.getElementsByTagName("head")[0] || | ||
document.documentElement; | ||
head.insertBefore(script, head.lastChild); | ||
script.textContent = code; |
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.
By retrieving the text and using textContent
, we can synchronously insert and remove the injected script
tag with no change of any hydration being confused by it.
Just wondering @phryneas: have you tried building the devtools locally with this fix and testing with sites where we see this issue? No problem if not, I can run the repro in a bit myself. |
@alessbell I've built them using |
SGTM @phryneas, let's add a changeset and 🚀 |
This would potentially fix #1162, #937 and #839
It's just a temporary fix though, as for Chrome we'll have to switch to manifest v3 soon, but it will be less of a problem there since that allows to inject content scripts into
ExecutionWorld.MAIN
.