Allow to apply arbitrary plugins when compiling ServiceWorker script #358
Conversation
Yeah, please, write tests. |
| @@ -30,6 +30,11 @@ export default class OfflinePlugin { | |||
| AppCache: false | |||
| }); | |||
|
|
|||
| if (options.ServiceWorker && options.ServiceWorker.plugins) { | |||
There was a problem hiding this comment.
This should be done in ServiceWorker.js file
There was a problem hiding this comment.
It cannot be because it refers to options, not this.options.
There was a problem hiding this comment.
That sounds wrong. ServiceWorker options shouldn't be touched in index.js, they all are handled in ServiceWorker.js file.
There was a problem hiding this comment.
I have to clarify a bit more. It uses deepExtend to set up this.options, and addTool passes it to ServiceWorker.js. However, deepExtend breaks this.options.ServiceWorker.plugins and ServiceWorker.js can do nothing with it when received. Therefore it has to be fixed beforehand.
There was a problem hiding this comment.
Yeah, I see what you mean now. It should be handled in some other way though. I'll merge this and do the change to this part myself.
(tests seem to fail now because master has been updated)
|
I'll be happy to merge this once everything is fixed. |
I'd like to use this feature to embed some environment variables in my custom entry module with
EnvironmentPlugin. I have optionalCDN_HOSTenvironment variable to determine an object storage in use. The entry module should have the variable, and be able to cache files served from the host.There could be other use cases, considering the wide use of
process.env.NODE_ENV. It is nice to have.Question: I have not implemented a test case, but should I? Should I place it at
tests/legacyortests? (I'm not sure whatlegacymeans.)