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

ServiceWorker throws errors because loadersMap is undefined #135

Closed
mstange opened this issue Nov 25, 2016 · 14 comments
Closed

ServiceWorker throws errors because loadersMap is undefined #135

mstange opened this issue Nov 25, 2016 · 14 comments

Comments

@mstange
Copy link
Contributor

mstange commented Nov 25, 2016

I'm using 4.5.0-beta.5 and am getting errors during the service worker installation. My __wpo does not contain a "loaders" key, so loadersMap is undefined, so Object.keys(loadersMap) fails.

I'm using the beta version because version 3 does not allow absolute paths and because 4.4.1 contains the wrong bit new URL(externals[path], location) which was fixed in e323307.

mstange added a commit to mstange/offline-plugin that referenced this issue Nov 26, 2016
mstange added a commit to firefox-devtools/profiler that referenced this issue Nov 26, 2016
mstange added a commit to firefox-devtools/profiler that referenced this issue Nov 26, 2016
@NekR
Copy link
Owner

NekR commented Nov 26, 2016

Hey @mstange, many thanks for the PR and for finding this issue. Main reason why I don't always expose loaders to __wpo is to not update all existing tests for that. I think the best way to fix it would be to just add || {} in the end of this line: https://github.com/NekR/offline-plugin/blob/v4/src/misc/sw-template.js#L13. I'll fix it now.

It's indeed unfortunate that you had to use beta version (which I actually tag unstable on npm).

NekR added a commit that referenced this issue Nov 26, 2016
@NekR
Copy link
Owner

NekR commented Nov 26, 2016

Okay, I committed it. @mstange can you check if it works now for you?

@mstange
Copy link
Contributor Author

mstange commented Nov 26, 2016

Thank you very much! The service worker works now.

I still get a reference error complaining about an unchecked options.allowLoaders access, but I'm not sure how fatal that is. ('ReferenceError: reference to undefined property "allowLoaders"', in addAllNormalized)

It's indeed unfortunate that you had to use beta version (which I actually tag unstable on npm).

Since version 3 is not an option for me, would it help if I downgraded to 4.4? I can do that if you fix that other bug that I ran into (the one with externals[path] - I can file a separate issue about that if you want).

@mstange
Copy link
Contributor Author

mstange commented Nov 26, 2016

Ah, the allowLoaders thing is just a warning, not an error.

@NekR
Copy link
Owner

NekR commented Nov 26, 2016

('ReferenceError: reference to undefined property "allowLoaders"', in addAllNormalized)

That's weird because I don't see anything like this. Do your use Flux or TypeScript or maybe just eslint and run it against generated output of your project?

@NekR
Copy link
Owner

NekR commented Nov 26, 2016

Since version 3 is not an option for me, would it help if I downgraded to 4.4? I can do that if you fix that other bug that I ran into (the one with externals[path] - I can file a separate issue about that if you want).

I'll release 4.5 out of beta soon. Need to solve/test some other things before that.

@mstange
Copy link
Contributor Author

mstange commented Nov 27, 2016

I have tracked down why I was seeing the allowLoaders warning: I was testing in a Firefox instance which I had launched using the Firefox add-on SDK's jpm command line tool, which by default sets the javascript.options.strict preference to true, which means that Firefox will print "strict mode" warnings.

@NekR
Copy link
Owner

NekR commented Nov 27, 2016

Okay, closing this now then :-)

@NekR NekR closed this as completed Nov 27, 2016
@NekR
Copy link
Owner

NekR commented Nov 29, 2016

@mstange if something v4 just shipped to master now :-)

@mstange
Copy link
Contributor Author

mstange commented Dec 12, 2016

Great! It's working very well for me now, after I figured out how I could redirect all unknown URLs to index.html using cacheMaps.

@NekR
Copy link
Owner

NekR commented Dec 12, 2016

@mstange yeah, cacheMaps isn't clearer API right now. It incrementally will get better. Do you have any feedback about it?

@mstange
Copy link
Contributor Author

mstange commented Dec 12, 2016

I think all it needs is documentation, the API is pretty clear I think.

It might also be worth adding a isSinglePageApp switch that automatically adds a cacheMap that basically does what I'm doing here: https://github.com/mstange/cleopatra/blob/a99026fbca3c601b1f7fffad207c1d4611c88be9/webpack.config.js#L82

My only remaining problem with the ServiceWorker is that I'm not properly accepting the new ServiceWorker when I'm reloading the page in Firefox - sometimes I have to forcefully unregister the old ServiceWorker in about:debugging or I keep seeing the old cached assets.
I haven't debugged that yet. Maybe I'm using an API that no longer exists. My code is here: https://github.com/mstange/cleopatra/blob/a99026fbca3c601b1f7fffad207c1d4611c88be9/index.js#L19

@NekR
Copy link
Owner

NekR commented Dec 14, 2016

My only remaining problem with the ServiceWorker is that I'm not properly ...

I'll look into it a bit later, not enough free time this days.

@NekR
Copy link
Owner

NekR commented Dec 14, 2016

But thanks for feedback and everything, it's definitely appreciated, I'll just handle that later.

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

No branches or pull requests

2 participants