#4190: Fix content script readiness race conditions and context invalidation handling #4240
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #4240 +/- ##
==========================================
- Coverage 48.60% 48.58% -0.02%
==========================================
Files 890 890
Lines 26165 26175 +10
Branches 5392 5391 -1
==========================================
Hits 12717 12717
- Misses 12527 12536 +9
- Partials 921 922 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
fregante
commented
Sep 10, 2022
| void onContextInvalidated().then(() => { | ||
| unsetReadyInThisDocument(uuid); | ||
| console.debug("contentScript: invalidated", uuid); | ||
| }); |
There was a problem hiding this comment.
Part 1:
- revert #4058: fix race condition on tab load #4061
- remove "document ready" attribute
Demo
Screen.Recording.mov
Console sequence
The "invalidated" message here marks my "refresh extension" click:
| // `import()` is only needed to avoid execution of its dependencies, not to lazy-load it | ||
| // https://github.com/pixiebrix/pixiebrix-extension/issues/4058#issuecomment-1217391772 | ||
| // eslint-disable-next-line import/dynamic-import-chunkname | ||
| const { notify } = await import(/* webpackMode: "eager" */ "@/utils/notify"); |
There was a problem hiding this comment.
Part 2:
- fix the issue closer to the cause
Part 3, in the future:
2 tasks
BLoe
approved these changes
Sep 12, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Closes Content script remains "ready" even after unloading #4190
Mark document as "not ready" when the context is invalidated
Unload kbar on invalidation?
Also ensure that consecutive
ensureContentScriptcalls resolve at the same time, potentially viapMemoize(fn, {cache: false})Checklist