[WIP] Prototype adding direct support for AMP and PWA

[westonruter]

I noticed the instructions on How to Set up AMP Web Push for Wordpress sites, but it doesn't yet indicate how to integrate with the official AMP plugin. Would you consider wanting to add direct support for it? (I'm one of the maintainers of this plugin.)

Also, there is a PWA feature plugin which is going to be proposed for merging into WordPress core (for which I'm also a maintainer). This includes not only a centralized way to manage the web app manifest but more importantly it includes a way for themes and plugins to register scripts to include in a single-installed service worker. You can see in this PR how to integrate both with the PWA plugin's registered web app manifest as well as the service worker that it generates. The goal for this not only is to improve compatibility with other themes and plugins but it will also hopefully eliminate code that isn't core to your feature set.

I'm happy to help advise on how to integrate.

---

This change is 

[mj00712]

Hi, Hope you are doing well. How can I enable Web Push notification in Official Amp Plugin? AMP Web Push Setup
Will the "Non-WordPress guide" work for WordPress users using Official Amp Plugin?

[westonruter]

I appear to have made some incorrect assumptions about what is required to integrate OneSignal with AMP and the PWA plugin. Closing this PR in favor of the work being done in Automattic/newspack-plugin#417 which I believe is going to be proposed for merge back into this plugin.

[jkasten2]

@westonruter We removed the manifest.json requirement from our plugin. The only thing left here would be to make the OneSignal ServiceWorker compatible with other WordPress plugins that also use them. Such as the PWA plugin.

You noted that the PWA feature plugin has a feature to will merge all ServiceWorkers into one file and that it will be a part of WordPress core. Do you know if WordPress core has already add this? Or know what version of WordPress might introduce this API?

It looks like the API for the PWA plugin is add_action('wp_front_service_worker', ...) noted by these lines in your PR. This is something we can add to this OneSignal plugin however we still need a fallback if someone does not have the PWA plugin installed. Do you have any suggestions on detecting this?

[jkasten2]

@westonruter As far as AMP goes, you are correct in that there are no current conflicts that would not allow OneSignal AMP to be added to another plugin. Such as the newspack-plugin and the work being done in Automattic/newspack-plugin#417.

I'll take a look over the Automattic/newspack-plugin#417 PR and see if there is anything missing to integrate OneSignal in it. However the other question as you noted in your Automattic/newspack-plugin#417 (comment) is if it should just be upstream here. It depends on the kind of integration newspack wants with OneSignal. Not requiring this OneSignal-WordPress-Plugin would most likely make adding OneSignal easier for users of the newspack plugin. On the other hand they may need to make changes in the rare case something changes in the integration. Ether way we will take a look to prevent any conflicts.

It seems most of the work is done in the Automattic/newspack-plugin#417 PR, we would open to collaborating on getting a PR up to add AMP support to this plugin as well. @jeffersonrabb @westonruter We created a branch called amp_support we can all work from as a base.

[westonruter]

You noted that the PWA feature plugin has a feature to will merge all ServiceWorkers into one file and that it will be a part of WordPress core. Do you know if WordPress core has already add this? Or know what version of WordPress might introduce this API?

This is partly why I closed this PR. It turns out that there is no need to merge the service worker used by OneSignal for push notifications into the service worker used to control other aspects of the site. The most important thing is to ensure that the scope of the registered service worker point to some path that is unique for the HTML pages used by amp-web-push, outside of any scope that would be used by the site's main service worker to control other aspects of the site.

It seems most of the work is done in the Automattic/newspack-plugin#417 PR, we would open to collaborating on getting a PR up to add AMP support to this plugin as well. @jeffersonrabb @westonruter We created a branch called amp_support we can all work from as a base.

Yes. The latest status can be seen in that PR, so I think this here PR is obsolete.

[jkasten2]

@westonruter Thanks for the quick response. You are correct, multiple service workers can be installed on one domain however, only if the scope for them ins't the same.

Since this plugin and the default recommendation for OneSignal is to install it's server worker as the root scope I am looking on how to prevent other service workers from replacing ours, and vice versa. It seems at least your wp_front_service_worker on your PWA feature plugin seems to be working to solve this problem. I also found your discussion on getting this added to the core WordPress and a w3c thread on allowing this on the browser level. However these do not seeing be getting any traction.

So it seems at least for the WordPress ecosystem adding your wp_front_service_worker if it is available is the best option currently to try to prevent some of these service worker replacement issues. Now just need to found out what is the best way to detect if the wp_front_service_worker action is available. 😃

[westonruter]

Since this plugin and the default recommendation for OneSignal is to install it's server worker as the root scope

Why is this the default/recommendation? Why not use a custom scope unique to OneSignal? Then there would be no conflict with other service workers.

[jkasten2]

I believe if you want the page to be able use navigator.serviceWorker.controller the page needs to be in the service worker's scope. Otherwise navigator.serviceWorker.controller will be null.

OneSignal's page SDK uses navigator.serviceWorker.controller.postMessage to talk to OneSignal SDK service worker to do things like query it's version (to update it if needed), to make background REST requests (to ensure they are completed between tab closes / page loads), and if there are any other tabs open to the domain (used to know if a page should be refocus on a notification open or a new tab opened).

[westonruter]

@jkasten2 I don't think that is the case. You can still send messages to other service workers. You just have to send them to their registrations, rather than using navigator.serviceWorker.controller.

Here's a proof of concept: https://glitch.com/edit/#!/noiseless-special-titanium

There's one thing that I'm not currently able to get to work in this, and that is sending messages from the uncontrolling service worker back to the page (I'm getting advice on why this is the case). Update: See #187 (comment).

However, you can see that it is able to successfully send messages from the page to the uncontrolling service worker (in a scope outside the page's URL). And this is apparently all you need. If so, then this would allow you to operate entirely independently of any other service worker that is controlling the page, and you'd be able to greatly improve interoperability across sites that are looking to integrate OneSignal.

[jkasten2]

@westonruter Thanks for digging in to this! It will definitely be awesome if we can find a way to remove the server worker scope requirement and keep all our features working. To do this I think we need to take inventory of what we are requiring the scope for today and if their is an alternative API or alternative strategy.

I created OneSignal/OneSignal-Website-SDK#615 to continue this discussion, since this is going out of scope of just this WordPress plugin. We can use this new issue thread for the inventory and status of each item of requirement on the scope the SDK has.

[westonruter]

There's one thing that I'm not currently able to get to work in this, and that is sending messages from the uncontrolling service worker back to the page (I'm getting advice on why this is the case).

Thanks to advice from @jeffposnick, this is fixed now as well in the demo: https://noiseless-special-titanium.glitch.me/

You can see the page successfully communicating bidirectionally with an uncontrolling service worker: