-
Notifications
You must be signed in to change notification settings - Fork 828
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
Prod error: Unable to import module 'workbox-google-analytics' #1186
Comments
I'm able to reproduce this locally with a importScripts('https://storage.googleapis.com/workbox-cdn/releases/3.0.0-alpha.5/workbox-sw.js');
workbox.setConfig({ debug: false });
workbox.googleAnalytics.initialize(); After unwinding the minification a bit to figure out which statement is actually causing the syntax error, it looks like it's the const e = yield c = o.body, new Promise((e, t) => {
const n = new FileReader;
n.onloadend = (() => e(n.result)), n.onerror = (() => t(n.error)), n.readAsText(c)
}); The corresponds to the following two blocks of unminified code: workbox/packages/workbox-google-analytics/_default.mjs Lines 41 to 48 in 40c2ac2
It seems like the process of running this code through the async-to-generator transpilation followed by UglifyJS minification has led to minified code that contains a syntax error. @philipwalton, do you have some bandwidth to investigate why that usage in the original source might be minified incorrectly, and potentially rewrite the code to avoid using (There are no in-browser integration tests for |
@philipwalton if you don't have bandwidth for this - please let me know I'll pick it up. |
@gauntface if you have time, that would be great. I'm trying to get two other projects out the door before the M64 release. |
I'm happy to pick up the fix, too 😄 I just figured I'd give Phil first crack at it. Testing things out locally, it looks like explicitly flagging (Thanks for filing #1193 to track the integration test, Matt.) |
I'm still seeing this issue with Workbox 3.5.0 in Firefox 62 but not in Chrome 69. You can see this at https://beta.basketball-gm.com/
|
Hmmm, @dumbmatter I'm not seeing that error when I run your site in FF 62.0 on Mac OS 10.13.6. What system are you running it on? |
I use Ubuntu, here is a screenshot of what I see: I have a Browserstack account so I tried Firefox 62 on Mac High Sierra and Windows 10, and everything worked fine there. How strange! Here is the contents of https://storage.googleapis.com/workbox-cdn/releases/3.5.0/workbox-google-analytics.prod.js when I load that directly in Firefox:
|
Okay after writing that I realized I'm being stupid - it's being blocked by my ad blocker (uBlock Origin). However, it is not a great for Workbox if the default behavior of a very common ad blocker is to completely block all service workers using this feature, rather than simply blocking Google Analytics tracking. |
Hmmm, seems like it's worth filing a bug with uBlock Origin. I agree it's not great for Workbox, but at this point we can't just change the name of our module (presumably uBlock Origin is just blocking anything with the name "analytics" in it?), so it seems like the best thing to do is for uBlock users to complain about the behavior. |
That's probably true, but it's fighting an uphill battle to try to prevent every ad blocker from ever blocking it. Is there any way to try/catch the import of workbox-google-analytics.prod.js, so you could still load the service worker even if some HTTP request failed? Or would that cause more problems, by leaving some users with an incomplete service worker in perpetuity? All this makes me wonder if I would be better off just bundling Workbox into my sw.js file, to prevent any issue like this in the future. That's what I do for all my JS dependencies outside the service worker. |
We could try/catch the import, but then your code would still error as soon as you referenced We could go even a step further and add a proxy to the namespace if the import failed. That might work in the Google Analytics case, but it wouldn't work in the general case. @jeffposnick do you have any thoughts here as to whether it's worth special casing the Google Analytics
So, this is exactly what I do for my personal website. I reference all the workbox modules using ES module import syntax and then bundle it with Rollup. Here's an example on a development branch: |
Hmm, that's tricky. I don't know how we could cleanly handle this given the complications mentioned above—the workarounds are not great. What if we just renamed the |
We could try renaming the script (assuming that works). I'm not crazy about it, but it's probably worth avoiding issues like this if we can. I'm moving this to #1646 to track it separately. |
Library Affected:
3.0.0-alpha.5
Browser & Platform:
Version 63.0.3239.108 (Official Build) (64-bit) - PC
Issue Description:
Use workbox google analytics:
On dev mode it works, but when I'm switching it to production mode:
I get the following error:
Additional info:
workboxServiceWorker.js line 60 is the line of workbox.googleAnalytics.initialize(); command in my service worker.
The text was updated successfully, but these errors were encountered: