-
Notifications
You must be signed in to change notification settings - Fork 22
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
Improve logging around contentScript
loading/deduplication code
#5025
Changes from all commits
68ba430
2277d64
d7f5831
ebff8c1
b69074e
201f8f8
7ef6114
6dd5e62
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,6 @@ import "@/development/visualInjection"; | |
import { uuidv4 } from "@/types/helpers"; | ||
import { | ||
isInstalledInThisSession, | ||
isReadyInThisDocument, | ||
setInstalledInThisSession, | ||
setReadyInThisDocument, | ||
unsetReadyInThisDocument, | ||
|
@@ -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}`; | ||
const uuid = uuidv4(); | ||
|
||
if (isInstalledInThisSession()) { | ||
console.error( | ||
"contentScript: was requested twice in the same context, aborting injection" | ||
`contentScript: was requested twice in the same context, aborting injection ${context}` | ||
); | ||
return; | ||
} | ||
|
||
if (isReadyInThisDocument()) { | ||
console.warn( | ||
"contentScript: injecting again because the previous context was invalidated" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was no longer reached because at the end of |
||
); | ||
} else { | ||
console.debug(`contentScript: injecting ${uuid}`); | ||
} | ||
console.debug(`contentScript: importing ${uuid} ${context}`); | ||
|
||
setInstalledInThisSession(); | ||
|
||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
setReadyInThisDocument(uuid); | ||
|
||
// eslint-disable-next-line promise/prefer-await-to-then -- It's an unrelated event listener | ||
|
@@ -70,11 +64,9 @@ async function initContentScript() { | |
|
||
if (location.protocol === "https:") { | ||
// eslint-disable-next-line promise/prefer-await-to-then -- Top-level await isn't available | ||
void logPromiseDuration("contentScript: ready", initContentScript()).catch( | ||
(error) => { | ||
throw new Error("Error initializing contentScript", { cause: error }); | ||
} | ||
); | ||
void initContentScript().catch((error) => { | ||
throw new Error("Error initializing contentScript", { cause: error }); | ||
}); | ||
} else { | ||
console.warn("Unsupported protocol", location.protocol); | ||
} |
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.
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.