-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Added options to allow for overrides to workbox-webpack-plugin #5369
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Still a WIP at the moment, have not verified the changes are working yet. Just looking for some validation on approach right now. |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
@Timer, I not sure what has caused the AppVeyor build to fail? I can see in the logs that a file is being imported from outside of the
However, I can see that same error message in the Travis build and it is passing? |
Been looking for this update... So confused how workbox was added b/c it is deemed the superior way to configure a PWA, but we have no ability to edit the Workbox webpack plugin that is being used, or do we? |
This is great, this is the final piece (set Is there anything pending on this PR that I can help with? Would love to have this merged for the next release. |
Looking forward to this getting merged. I am also looking forward to being able to set the |
This is the only show stopper for us as well. Looking forward to this merge. Can't stress how important skipWaiting is for UX. |
Really looking forward to this change as well! What's the status here? Does it need anymore work? If so, I'd be happy to help pitch in. Does anyone know why the tests are failing? |
@dstapleton92, I have not had the time to investigate the tests that are failing, other than AppVeyor seems to be failing on |
@mymattcarroll Yeah I see that. Doesn't look like it's from a change you introduced. Anyone have any ideas? |
@dstapleton92 seems a merge with the latest @Timer any chance you could take another look at this? |
What's the status on this ? do you need any help merging this ? |
@RxAssim
I assume that question was not for me? |
@mymattcarroll @Timer @iansu Does this look good to merge? My team would really like to re-enable the service worker for our application, but that's a no-go until we can configure it. |
I don't seem to have access to re-run the tests? Could someone with access try them again? |
You could always push a dummy file then remove, that should trigger the tests... |
So I've been unfortunately able to reproduce this error, going to see if we can fix it... |
Hi guys, Sorry to ask you this, but do you have a potential release date ? Thx a lot in advance, Olivier |
@oallouch in the meantime you may use |
@FezVrasta reading the source of the named customizer, it looks like this only allows to override constructor arguments for the Now regarding this PR, wouldn't it be better that it would be amended to allow for using the InjectManifest instead of the GenerateSW plugin? |
@rmoorman feel free to send a PR to update the customizer! I wasn't aware of |
@FezVrasta if I can find any time to do so I can give it a try. Besides that, that was not exactly my point. The problem I see with this, is that this (according to the workbox guys) does not easily allow to add custom logic and use browser apis such as web push as there still will be no way to use something else than GenerateSW out of the box. This PR does only account for passing other options to the GenerateSW plugin. Not for using another plugin altogether such as the recommended plugin allowing for custom logic in the service worker. It is not useful in my opinion (even though I can totally agree with the desire to keep configurability to a minimum in default CRA) that a whole additional configuration file would be introduced just to set a few options on a plugin which should enable "service workers" in CRA but does not seem to be well suited to really get going with things service workers provide besides caching. |
I am sad to see initial solutions to solve problems of several people taking too long to be accepted. @ianschmitz any progress to merge this or need help for this? This partially meets, but with it it would be simpler to evolve to the solution of using InjectManifest conform @rmoorman :) |
@mymattcarroll @iansu @ianschmitz I opened PR #8485 and with it it will be possible to make any type of configuration of the Workbox, I was careful and left it without any break. It will be possible to use the most advanced features of the Workbox using InjectManifest, as GenerateSW is very superficial and limited. |
Does anyone know when is this feature going to be released? Thx! |
I want to override workbox config too, any update..? 👀 |
Any update on this? It could probably solve #7700 which causes blank pages in production builds as I described here: GoogleChrome/workbox#2299 Should be high prio, it's affecting clients, I always need to navigate them to wipe browsers cache and service worker. Other solution is to get rid of service worker, but it will not be possible to preserve app update procedure (modal window, update on click) and it can lead to BE/FE inconsistencies then if everyone has different app version due to cache. |
@ianschmitz @mrmckeb @petetnt Is there any news on this? |
#9205 switch workbox to |
This should be closed, as it would revert the new, customizable |
Agreed, we can look at continuing to improve how we support service workers in the future - but the work that @jeffposnick did enables a lot of customisation. |
Closes #5359
Verified by:
yarn build
increate-react-app
root and ensuring service worker usesoptions
return byworkbox.config.js