Skip to content
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

Support Manifest V3 #283

Closed
wants to merge 4 commits into from
Closed

Support Manifest V3 #283

wants to merge 4 commits into from

Conversation

0xBigBoss
Copy link

Our extension isn't live on chrome yet, but I've had success with these changes due to the fact the service worker is transient and may not be started immediately for content scripts/popup/etc.

Also, dispatching any event before subscribing to changes signals to chrome to start the service worker again.

const store = new Store()
await store.ready().then(async () =>
  await store.dispatch({ type: '' }) // wakeup service worker
)

Fixes #244

this.port = this.browserAPI.runtime.connect(this.extensionId, { name: this.portName });

this.port.onDisconnect.addListener(() => {
setTimeout(() => this.connect(deserializer, ++attempts), 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I attempted this approach, the major problem was that the background script stopped after processing the connection request.

Therefore, it causes the following problems:

  • The store fails to sync after the reconnects is reached
  • The resource consumption of the extension significantly increases as the reconnect applies to each tab.

May I ask how you managed to solve these issues that I couldn't overcome with this architecture?

Copy link
Author

@0xBigBoss 0xBigBoss May 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I attempted this approach, the major problem was that the background script stopped after processing the connection request.

Yes, you cannot expect the service worker to be alive, they are transient. I have experienced a bug where two service workers may be started and the extension fails to sync (typically when the devtools are open). We have had no issues re-connecting in other cases though and the extension always restarts the service worker without fail.

The store fails to sync after the reconnects is reached

I have not seen this, although our extension right now is not running through content scripts. Only the popup.html, but if i have multiple popup.html open, stopping the service worker manually is handled gracefully on all tabs and they begin re-syncing state.

The resource consumption of the extension significantly increases as the reconnect applies to each tab.

I don't see how this increases resource consumption significantly other than the CPU to restart the connection. Set the maxReconnects to something smaller and handle in the script when re-connect fails.

May I ask how you managed to solve these issues that I couldn't overcome with this architecture?

Anything that needs to survive past service worker restarts must be persisted to disk either e.g. chrome.storage.local or for sensitive data use chrome.storage.session

Hope that helps!

Copy link
Contributor

@eduardoacskimlinks eduardoacskimlinks Jun 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this approach and the nature of the service worker will conflict with our extension for the downside highlighted in my previous post, so we went for a different architecture as described on #282.

However, I am glad that you manage to sort your problems out through a different route. Please keep us informed if your user experiences any issues, we will do the same.

@0xBigBoss
Copy link
Author

Our extension is finally live running with this pull request, and with around 80 users only one saw the service worker not starting correctly. Will keep you posted.

Chrome extension if you're interested. https://chrome.google.com/webstore/detail/pokt-wallet/adganlhbinonbpfiehjjpmklkbghkaio?hl=en

@eduardoacskimlinks
Copy link
Contributor

@0xBigBoss, I saw you close the PR. How did it go with your approach to support Manifest V3? Did you see any issues that drove you to close the PR?

@0xBigBoss
Copy link
Author

@0xBigBoss, I saw you close the PR. How did it go with your approach to support Manifest V3? Did you see any issues that drove you to close the PR?

Just doing some housekeeping, it's not good IMO to leave PRs open for a long time.

So far without any updates the reconnect approach has been working as expected. Though there are still some edge cases where a client needs to manage the service worker lifecycle correctly and does require a refresh of the page. I'm not well-versed in the service worker architecture though to suggest a solution or how to handle it programmatically.

@eduardoacskimlinks
Copy link
Contributor

@0xBigBoss Makes sense, I am glad to hear it is working fine.

I have had a similar experience with my solution of continuously broadcasting state changes to all content scripts instead of reconnecting.

I feel that our challenges are related to the service worker's behaviour, as you pointed out. For example, I saw a few reports in the google chrome discussion group that pointed out a problem with issues in the implementation from the Google side. Hopefully, they will keep improving on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Manifest V3
2 participants