Fix back/forward caching issues on Safari #43
Closed
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
postMessage is now only used for initial handshake, once the ports are exchanged, the messaging then carries on over MessageChannel
Credits to @gurupras for finding the bug. Original pull request can be seen here - serversideup#26
Fixes serversideup#23 Each message sent now relies on delivery receipts to ensure they were indeed delivered AND to expect the destination's response. In case the destination disconnects before it could have responded, sendMessage Promise will be rejected with an 'Transaction ended before it could complete'. Queued messages are also no longer tracked in the background page, since it is ephemeral in manifest v3 extensions. Each endpoint now keeps track of its queued messages, and responses it is waiting on, in its on own runtime. This new strategy also acts as a safe-guard against destinations receiving stale messages from endpoints that no longer exist __(previsouly this could happen as there was no message invalidation logic for when the sender disconneted)__. The role of background page is no longer to maintain the "state" of the system, but to just act as a relay and to inform the relevant endpoints when the destination they are transacting with have been disconnected.
|
What do you think of returning the |
That would definitely make sense. I can probably take a look at it later this week. :) |
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.
Turns out issue #42 was cause by the use of BF cahing in Safari:
https://web.dev/bfcache/
BF caching takes the entire JS heap into a cache and reloads this instantly when using the back/forward buttons. The page state is restored from memory instead of being reloaded. This means that our connections would be disconnected when you navigate to a new page, and navigate back again. Example:
Thus the solution is to reconnect them, when the onpageshow event is called, if the persisted field is true.
@zikaari should this event be wrapped in a check for if window is defined in the context? I don't seem to be getting any runtime errors now, but maybe you know of any cases where it would be necessary?