Fix 2 codefactor warnings and remove whitespaces at end of lines of app.js #842
Conversation
mossroy
changed the title
mossroy
changed the title
| @@ -677,7 +677,6 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys | |||
| // If this is the first time we are initiating the SW, allow Promises to complete by delaying potential reload till next tick | |||
| delay = 0; | |||
| } | |||
| messageChannel = tmpMessageChannel; | |||
There was a problem hiding this comment.
I'm not sure I understand why this has been removed. Is it because we don't actually need to store this message channel?
There was a problem hiding this comment.
In fact, the global variable messageChannel was defined, assigned here, but never used.
I suppose we were keeping a single messageChannel instance at some point in the past, and afterwards we changed our mind.
So I removed the variable declaration, and assignment here.
There was a problem hiding this comment.
OK. At some point (not now) it might be good to check whether the implementation still needs the Service Worker to be kept alive. The Service Worker spec is specifically designed to put the SW to sleep when not being used, and it should fire up again immediately it is invoked again. It's possible, in fact, that an older implementation tried to keep the messageChannel alive (hence we have a global messageChannel assignment here), which should not be necessary. Each use of the SW can define a new messageChannel. I'm only speculating here, nothing tested!
There was a problem hiding this comment.
I doubt we could simply remove this keepalive implementation.
The SW can be removed and re-created by the browser, by design from the beginning.
The problem is that, when it's re-created, it does not have the messageChannel variable to ask app.js for the content. We send this messageChannel through the "init" message.
When I faced this issue in #145, I did not find a better solution.
But I'd be very glad to have a better one.
There was a problem hiding this comment.
OK, we can investigate another time. Fine to merge this if you've done some testing on SW mode.
mossroy
deleted the
remove-eol-whitespaces-and-fix-codefactor-warnings
branch
|
Sorry, commented too late! |
Removed unused variable messageChannel, and a weird character in a comment