Improve logging around contentScript loading/deduplication code
#5025
Conversation
Codecov Report
@@ Coverage Diff @@
## main #5025 +/- ##
=======================================
Coverage 59.67% 59.68%
=======================================
Files 968 968
Lines 29261 29258 -3
Branches 5599 5598 -1
=======================================
Hits 17462 17462
+ Misses 11799 11796 -3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| ); | ||
| return; | ||
| } | ||
|
|
||
| if (isReadyInThisDocument()) { | ||
| console.warn( | ||
| "contentScript: injecting again because the previous context was invalidated" |
There was a problem hiding this comment.
This was no longer reached because at the end of initContentScript we're already marking the document as not ready
| @@ -30,22 +29,17 @@ import { onContextInvalidated } from "@/errors/contextInvalidated"; | |||
|
|
|||
| // See note in `@/contentScript/ready.ts` for further details about the lifecycle of content scripts | |||
| async function initContentScript() { | |||
| const context = top === self ? "" : `in frame ${location.href}`; | |||
There was a problem hiding this comment.
Before
Without this piece of information, it seems that there are multiple injections in the same page:
After
This makes it clear that there are multiple frames. I'm only logging this piece of information one to avoid too much noise. The console already allows filtering by context, should you wish to debug it further.
| @@ -58,7 +52,7 @@ async function initContentScript() { | |||
| void logPromiseDuration("contentScript: imported", contentScript); | |||
|
|
|||
| const { init } = await contentScript; | |||
| await init(uuid); | |||
| await logPromiseDuration("contentScript: ready", init()); | |||
There was a problem hiding this comment.
In case of duplicate injection, the previous setup was logging "ready" after an error. Moving the logging fixes it.
|
@twschiller ready for merge. No change of behavior was seen |
|
This still contains additional code/logging improvements and can be merged separately. The only overlap is the dependency itself |
|
@fregante you'll need to resolve the merge conflicts |
fregante
changed the title
webext-dynamic-content-scriptscontentScript loading/deduplication code
I'm sending this PR to test out this new base in preparation for further fixes related to the injection of the content scripts. Here I think the only behavior change is:
Related
Not included / not fixed
ensureContentScriptfails on *.pixiebrix.com after an extension reload #4189content-scripts-register-polyfillto v4 fregante/webext-dynamic-content-scripts#54Tasks
*.example.comand*://*/*together