ember-cli / rfcs

Archive of RFCs for changes to ember-cli (for current RFC repo see https://github.com/emberjs/rfcs)
45 stars 54 forks source link

Add a Service Worker with minimal asset caching to a default Ember CLI application #117

Closed ghost closed 5 years ago

ghost commented 6 years ago

Rendered

Changelog:

2018-03-17

mikkopaderes commented 6 years ago

I think being able to configure additional assets to cache as part of the minimum should be included. Is there a reason why that was left out?

bcardarella commented 6 years ago

I believe that https://github.com/ember-cli/rfcs/pull/106 should be solved before this RFC is implemented. Having good ssl ergonomics will be essential to the dev on boarding of SW. Otherwise ember-cli is asking each dev to manage ssl on their own which is a nightmare

ghost commented 6 years ago

@bcardarella Service Workers don't require HTTPS on localhost. Meaning that you would only need said certificates if you run your Ember CLI app locally on a domain which is not localhost.

cibernox commented 6 years ago

How does this play with ember-service-workers. Does this basically mean that ember-service-worker and ember-service-worker-asset-cache will it be part of the default blueprint or that they will be "merged into core"?

ghost commented 6 years ago

I intentionally left out the implementation details, but my intention is to add the required ember-service-worker addon and it's plugins as a part of the default blueprint.

EDIT: an alternative could be a 'good defaults' meta-addon, that packages said combo of addons with the correct configuration preset.

knownasilya commented 6 years ago

Having good ssl ergonomics will be essential to the dev on boarding of SW. Otherwise ember-cli is asking each dev to manage ssl on their own which is a nightmare

I want to second what @bcardarella voiced, because now-a-days I find myself having to setup a local domain via etc/hosts either to handle multitenant apps with subdomains or for oauth testing.

rtablada commented 6 years ago

@martndemus I like the idea around this, but I worry about the invalidation and versioning complexity.

+1 for having an opt out that when disabled a script to deregister service worker.

mikkopaderes commented 6 years ago

After thinking about this. I'm on the side that this should be an opt-in thing. There's just too much GOTCHA around this to be turned on by default.

piotrpalek commented 6 years ago

I agree, I think making it opt-in by using a generator shipped with Ember CLI would be a good approach with this. After it had time to settle and shake out any potential bugs or DX downgrades it could be enabled in the next major Ember version.

ghost commented 6 years ago

but I worry about the invalidation and versioning complexity.

There's just too much GOTCHA around this to be turned on by default.

After it had time to settle and shake out any potential bugs or DX downgrades it could be enabled in the next major Ember version.

I hear your concerns, and I admit that there are some ways to troll yourself with Service Workers. But I am very confident in that we can ship this minimal SW implementation without any major gotcha's, DX downgrades, etc.

If you have a specific concern, I'd love to hear about it and I'm sure there is a good solution for it.

mikkopaderes commented 6 years ago

@martndemus - It's been a while since I've worked with PWA since the last one I was developing on got abandoned. But if I was remembering it correctly, the main problem was I was accessing my localhost from another mobile device to test out the app. So in my ipad/iphone/android/etc it was like http://<local_ip_address>:4200. I would always have to remove the cache on those devices to see my changes. I think private browsing solves it but I was using some library that wouldn't work on private browsers.

EDIT: I could've always turned off SW back then but on the chance that I forgot it, damn. Erasing just one specific cache is impossible at least on Android. I had to remove my whole browsing history for those devices.

kylemellander commented 6 years ago

I know that there is the polyfill for service workers that brings support up for browsers that do not support service workers yet, but I know for one of my use cases, we were using android/ios webviews for apps and the service worker polyfills did not work properly. I know that Apple recently added support for service workers on their mobile webview, but I just wanted to raise the concern about potential areas of lack of support that would cause issues with devs.

cibernox commented 6 years ago

@kylemellander service workers are designed as progressive enhancements. If you're developing for a browser/webview that doesn't support them, all will continue to work as it does today. It's only an improvement for browser who do.

ghost commented 6 years ago

Updated according to various feedback. See OP for changelog.

courajs commented 6 years ago

I don't think this should be on by default. Currently if an app is designed with zero thought about offline support, it will simply not load while offline. This communicates clearly to a user that this app shouldn't be expected to work offline.

If a service worker is installed by default, I believe a lot of apps will still be designed with zero thought about offline support. But now, the app will appear initially appear to work offline to a user, before breaking in arbitrary and confusing ways as the developer's assumptions about connectivity are violated.

I think a default-installed service worker would lead to an overall more confusing world wide web.

I would not be opposed to adding a --service-worker or --offline-capable flag to ember new, since that would be opt-in.

knownasilya commented 6 years ago

What about a default that displays an offline page?

piotrpalek commented 6 years ago

We definitely should also consider that CRA is moving away from having service workers on by default. It's a strong signal that there were issues with it despite efforts to make it non invasive: https://github.com/facebook/create-react-app/issues/2554

simonihmig commented 6 years ago

I would also ask not to do this by default. Mainly because of my concern that users unaware of it could be trolled by its side effects. Although I had checked this reload checkbox in Chrome, at least once Chrome was messing things up, having a list of SW registrations queued for activation, but nothing ever happened. So my browser was only seeing stale assets, everything I changed in my project did not reflect in the runtime. It took me a while to figure this out, which required me to manually unregister the SW and clear all site data. And this although I knew we had a SW in place. Now imagine someone isn't aware of this at all, and how to fix it!

@martndemus I know the recent strategy change (cache-fallback instead of cache-first) would mitigate that problem, but I am not sure this is a good default. I think in production cache-first still makes sense. But for dev, we actually disable SW all together by default (i.e. make in opt in, to work with SW only if you're actually working on SW related things). But this kind of setup would probably be even more dangerous if you are not fully aware of it.

tl;dr Adding a SW should be a conscious choice by the user!

ddoria921 commented 6 years ago

I think this feature will end up backfiring new users who might not expect this behavior from the very beginning. It can lead to a frustrating first experience with Ember since you'll have to check the reload box in Chrome for changes to take effect.

I don't think this should be on by default, but I would like to see it available as part of a flag when you're starting a new project. Like @simonihmig said, it should be a conscious choice by the user. By having it as a flag on setup, a user would benefit by out-of-the-box support for service workers, but only after explicitly opting into it.

kellyselden commented 6 years ago

We could start thinking about a ember new --interactive with a yeoman style list of choices. This is where our "approved defaults" that take additional thought and configuration could live. This could also answer the what to do with ember-data question.

simonihmig commented 6 years ago

We could start thinking about a ember new --interactive with a yeoman style list of choices.

This is exactly the kind of idea I had in my mind for quite some time. Just didn't find the time yet to dive deeper into it, not to speak of writing a RFC. Something like...

$ ember new --interactive
What kind of project do you want to create?
● default
○ minimal
○ PWA
○ SSR (FastBoot)
○ Native Mobile (Cordova)
○ Native Desktop (Electron)

It's a bit off-topic here, but could indeed solve the issue of supporting more opinionated stuff! Happy to help push this idea forward, if that sounds reasonable!?

btecu commented 6 years ago

@simonihmig Oldest pull request open was doing something related. It never got completed: https://github.com/ember-cli/ember-cli/pull/4709

knownasilya commented 6 years ago

@simonihmig https://github.com/ember-cli/rfcs/pull/7

eriktrom commented 6 years ago

per @kellyselden's comment

We could start thinking about a ember new --interactive with a yeoman style list of choices

https://workboxjs.org/ is worth looking into further. It's basically what we want now, and gives power users more control, without a foot gun for new users. We could hook into their api instead of inventing our version, but with perhaps nicer conventions, a la --interactive for ember apps.

It's specifically designed to deal with many of the worries/concerns/excitement around this web feature.

just a thought

eriktrom commented 6 years ago

service worker guides here are here within workbox

mike-north commented 6 years ago

There are still some seriously rough edges around service workers. I know of an internet-scale web app that's having to temporarily roll back serving up a SW to chrome users due to this bug which causes IPC between frames to blow up (Chrome 65): https://bugs.chromium.org/p/chromium/issues/detail?id=822145

While the idea of having a "pwa by default" or "offline first" experience is enticing, there's a whole lot of foot-gun factor involved today, some of which can prevent affected users from accessing a domain entirely for 24 hours or more. I really want to see us get there -- we just have to decide how many rough edges are acceptable.

oskarrough commented 6 years ago

tl;dr Adding a SW should be a conscious choice by the user!

Fully agree, considering the gotchas that are still here. Why not tell people to ember install ember-service-worker instead?

mikkopaderes commented 6 years ago

@oskarrough although I do agree that SW shouldn't be on by default, I do would like to get built-in support for SW should we opt-in. That's one less package to worry about when maintaining our app dependencies.

NullVoxPopuli commented 6 years ago

I think a default service worker implementation needs to have a way to auto-detect updates and prompt the user for refresh. There is an existing ember-server-worker addon for this out there, but it doesn't work 'automatically' (at least for https://github.com/NullVoxPopuli/emberclear anyway...).

been trying this hack:

    setInterval(() => {
      navigator.serviceWorker.getRegistration()
        .then(registration => registration && registration.update());
    }, 60000);

which still doesn't quite get me there.

See: https://github.com/topaxi/ember-service-worker-update-notify/issues/2

rwjblue commented 5 years ago

We are working on closing the ember-cli/rfcs repo in favor of using a single central RFC's repo for everything. This was laid out in https://emberjs.github.io/rfcs/0300-rfc-process-update.html.

Sorry for the troubles, but would you mind reviewing to see if this is still something we need, and if so migrating this over to emberjs/rfcs?

Thank you!

NullVoxPopuli commented 5 years ago

Moving this to the main RFC repo would be cool. Also, having default update notification. ECS-update-notify does that through ember concurrency, so, that may be too many deps