-
Notifications
You must be signed in to change notification settings - Fork 873
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
Fixing a missing defensive check for an absent window global variable that was breaking service worker environments #443
Conversation
… that was breaking service worker environments
This fixes my issue 🥳 I'm looking forward to the merge! |
I think this fixed my issue too, would be great to get it merged since it's approved |
It seems this PR has not been released yet. |
Any chance this could get released soon? 😁 |
This was merged in a while back now but there still hasn't been a release. Any idea when this might happen? Lots of web extensions are having to move to manifest V3 in the next few months and one of the big changes there on Chrome is that your extension's background script will now run as a Service Worker. Which lacks a |
I've been using this pnpm patch / patch-package in lieu of a release: https://gist.github.com/rory-instil/2091eeeaf087d6caaff7c91a7e16f384 |
Released v7.1.3. Please try it out and let me know if the issue is fixed. |
Just verified it fixes it for me in Chrome MV3 background service worker context. Thanks @martincizek ! |
I spoke too soon without giving it a good enough test. After properly testing it, it errors out trying to access I see you're using It would also probably also significantly increase the SW script size. An ideal solution would be to afford the user passing in their own HTML parsing implementation, as that may allow users to avoid the script size increase if they're already using some other non-browser HTML parsing solution, like |
There are different builds for browser and non-browser. I think you can influence this with how you declare the dependencies. And the online browser-build does not contain domino indeed. |
This code should not be accessed if you provide |
Ah that's quite obvious in hindsight. Seems to work fine with a |
Reusing the
root
variable defined in the file above - and used in a similar way on line 12 - we can enable service worker environments to use this library.Otherwise these environments will encounter a
window is not defined
error.This will close #397.