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

amp-install-serviceworker: Trigger event when service worker has been update has been installed #18615

Open
westonruter opened this issue Oct 8, 2018 · 13 comments
Labels

Comments

@westonruter
Copy link
Member

westonruter commented Oct 8, 2018

At the moment there is no way to alert the user when an updated service worker has been installed. Often pages inform the user of this with a prompt such as:

image

I suggest that amp-install-serviceworker be extended to add support for an updatefound event which can be triggered on the amp-install-serviceworker element.

function install(win, src) {
return win.navigator.serviceWorker.register(src).then(function(registration) {
if (getMode().development) {
user().info(TAG, 'ServiceWorker registration successful with scope: ',
registration.scope);
}
return registration;
}, function(e) {
user().error(TAG, 'ServiceWorker registration failed:', e);
});
}

Perhaps instead of updatefound it would be better to name it updateinstalled since as I understand we're primarily interested in when an updatefound event results in 'installed' === registration.installing.state being true. So perhaps something like so:

function install(win, src) {
  return win.navigator.serviceWorker.register(src).then(function(registration) {
    if (getMode().development) {
      user().info(TAG, 'ServiceWorker registration successful with scope: ',
        registration.scope);
    }

    reg.addEventListener('updatefound', () => {
      reg.installing.addEventListener('statechange', () => {
        if ( 'installed' === registration.installing.state ) {
          const name = 'updateinstalled';
          const event = createCustomEvent(this.win, `${TAG}.${name}`, dict({}));
          Services.actionServiceForDoc(this.element).trigger(this.element, name, event, ActionTrust.HIGH);
        }
      });
    });

    return registration;
  }, function(e) {
    user().error(TAG, 'ServiceWorker registration failed:', e);
  });
}

With something like this in place, an author could then show a notice using amp-bind like so:

<amp-state id="updateAvailable">false</amp-state>
<amp-install-serviceworker 
  on="updateinstalled:AMP.setState({updateAvailable:true})" 
  src="https://www.example.com/serviceworker.js"
  data-iframe-src="https://www.example.com/install-serviceworker.html"
  layout="nodisplay"
></amp-install-serviceworker>
<button hidden [hidden]="! updateAvailable" on="tap:AMP.navigateTo(url=AMPDOC_URL)">
   Update available!
</button>

(Maybe there should be an AMP.reload() action available as well.)

Thoughts?

@westonruter westonruter changed the title amp-install-serviceworker: Trigger event when service worker has been updated amp-install-serviceworker: Trigger event when service worker has been update has been installed Oct 8, 2018
@westonruter
Copy link
Member Author

/cc @cramforce

@kristoferbaxter
Copy link
Contributor

Added @ericlindley-g as he's been thinking about enhancements to amp-install-serviceworker for a little bit.

@cramforce
Copy link
Member

I wonder if we should establish a standard mechanism to update from amp-state from any worker. @kristoferbaxter Could it be the same API as what we're planning to use for amp-script?

@kristoferbaxter
Copy link
Contributor

Agreed @cramforce.

We should have the same standard, I'll write something up.

@westonruter
Copy link
Member Author

westonruter commented Oct 8, 2018

I wonder if we should establish a standard mechanism to update from amp-state from any worker.

❤️ I was wanting to write up something about this as well. In particular I had in mind: if a service worker is using a stale-while-revalidate strategy for caching navigation requests, as soon as revalidation results in there being an update to a cached response, the service worker needs to communicate a message to the client that there is an update to that URL and the user should be prompted to refresh.

I'm sure you have something in mind already, but what comes to mind for me is a message event that is triggered on the amp-install-serviceworker element which could then be used in a call to AMP.setState(). For example:

<amp-state id="stateFromServiceWorker">{}</amp-state>
<amp-install-serviceworker
	on="message:AMP.setState({stateFromServiceWorker:event.data})"
	src="https://www.example.com/serviceworker.js"
	data-iframe-src="https://www.example.com/install-serviceworker.html"
	layout="nodisplay"
></amp-install-serviceworker>

Where messages could be sent from the service worker which could then populate the state via the event handler.

@cramforce
Copy link
Member

I was thinking about something more imperative. And giving the SW itself access to the page's state.

@kristoferbaxter
Copy link
Contributor

I was thinking about something more imperative. And giving the SW itself access to the page's state.

That's where I was headed as well. I'll put the document up tomorrow.

@prateekbh
Copy link
Member

@kristoferbaxter do we have any doc for this already?

@prateekbh
Copy link
Member

Any changes to amp-state without an explicit user gesture is forbidden from updating any UI. We might wanna re-visit that clause

@nainar
Copy link
Contributor

nainar commented Aug 5, 2019

Assigning this to @choumx since we need the ability to pass messages between threads for amp-script too.

@stale
Copy link

stale bot commented Dec 19, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale Inactive for one year or more label Dec 19, 2021
@stale stale bot closed this as completed Dec 26, 2021
@westonruter
Copy link
Member Author

This is still desired.

@westonruter westonruter reopened this Jan 10, 2022
@stale stale bot removed the Stale Inactive for one year or more label Jan 10, 2022
@stale
Copy link

stale bot commented Jan 7, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale Inactive for one year or more label Jan 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants