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

Proposal: more advanced Service Worker support #7966

Open
MOZGIII opened this issue Nov 12, 2019 · 5 comments
Open

Proposal: more advanced Service Worker support #7966

MOZGIII opened this issue Nov 12, 2019 · 5 comments

Comments

@MOZGIII
Copy link

MOZGIII commented Nov 12, 2019

Is your proposal related to a problem?

Currently, the Service Worker support is very limited because the service-worker.js is generated by CRA and users can't customize it.

It's bad cause Service Workers can do much more than we currently use today, and by their inherent design, should allow app-specific logic.

Describe the solution you'd like

We're using workbox-webpack-plugin go generate service worker via it's GenerateSW.

Instead, we could switch to InjectManifest, and add a sample service worker file src/service-worker.js to all the app templates. Therefore users will be able to customize the behavior of the Service Worker by editing the src/service-worker.js file, and we can preserve the existing runtime defaults by putting the same logic at the src/service-worker.js template that GenerateSW generates by default.

Regarding TypeScript support, we should be able to transpile that service worker file too!

Additionally, we can benefit from this approach by disabling service worker generation logic if the src/service-worker.js is removed. This way we can solve long outstanding request to disable service worker generation.

Describe alternatives you've considered

I've looked into issues #5369 and #5359. They both essentially propose providing more flexibility by allowing webpack config customization. I think that would be a more generic solution, but it has some significant downsides:

  1. We'd have to support unbound set of build configurations. My proposal allows flexibility while only having a single configuration. This can be arguably a strong point, and I've seen it a lot in the decision making of this project before. Yet, personally, I don't think it's that problematic.

  2. We can provide better support using this proposal than by allowing webpack build configuration. By this, I mean that we can integrate the InjectManifest better if we only have that, in contrary to supporting both InjectManifest and GenerateSW, or even allowing users to provide an arbitrary plugin there. We can add the logic to disable service worker generation naturally, TypeScript support and possibly other built-time service worker injectors if we ever need to.

  3. This proposal will also simplify bug fixing cause there will be less possible configuration states that the user can put the build system into. This kind of repeats the first point (1).

The #7866 might provide flexible webpack build process configuration, and relieve the pain points with the current service workers support. However, this proposal is focusing on more advanced support for Service Worker - not just solving the current pain points - and, in my opinion, has value independently of whether #7866 (or similar) is accepted or not.

Additional context

I'll be able to submit a PR if this proposal is accepted.

@MOZGIII
Copy link
Author

MOZGIII commented Nov 21, 2019

Implemented a Draft PR, we can discuss that implementation it there (#8011) and the general idea discussion should be kept here.
I hope to hear from the core team.

@marlonmleite
Copy link

This is a really good thing. For the knowledge that I have in the Workbox, this would suit perfectly.

But, I think it is very important that if this idea goes forward, it should be done as soon as possible, because with each passing day it becomes more complicated to continue in the CRA without ejecting it. This is something of extreme urgency for those who work with Service Worker.

@marlonmleite
Copy link

Using InjectManifest, this would allow the following features in the CRA with Workbox:

  • API cache;
  • Configure SDK/libraries at the Service Worker level (Firebase, FCM, Analytics and more);
  • Push notification;
  • Background sync;
  • Custom rules for precache and assets;
  • And more...

It's a good idea and a big feature for CRA :)

@esetnik
Copy link

esetnik commented Feb 27, 2020

A bunch of us have been bitten by a service worker related bug in #8047 which was actually fixed in the template but since our projects were generated before the template was fixed, remains broken in our projects until manually updating. I wanted to chime in here to request that some consideration be made that the out-of-the-box create-react-app experience should ideally include a service worker configuration that can remain up-to-date and receive improvements automatically.

@9jaGuy
Copy link

9jaGuy commented Feb 14, 2021

@MOZGIII this should be good with #9141 implementation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants