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.

Sign up for GitHub

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

Jump to bottom

[FR] - Allow <amp-install-serviceworker> to register a SW hosted on a subdomain #18055

Closed
demianrenzulli opened this issue Sep 14, 2018 · 12 comments
Closed

[FR] - Allow <amp-install-serviceworker> to register a SW hosted on a subdomain #18055

demianrenzulli opened this issue Sep 14, 2018 · 12 comments
Assignees
@jridgewell
Labels
Category: PWA Stale Inactive for one year or more Type: Feature Request
Projects
PWA and Shadow AMP
Milestone
New FRs

Comments

@demianrenzulli
Copy link
Contributor

Due to validations inside the component for the data-iframe-src attribute and for the src attribute, it's not possible to register a SW if the AMP page is hosted on a different subdomain than the PWA.

For some sites, though, this configuration is not uncommon.
Examples include:

  1. Having AMP and canonical at the main domain (www.site.com), and the PWA and SW in a subdomain (pwa.site.com, app.site.com, m.site.com).

  2. Having AMP pages on their own subdomain (amp.site.com), canonical on the main domain (www.site.com) and the PWA on a different one (pwa.site.com, m.site.com, etc).

On these cases, the registration of the SW from the AMP page will fail with the current implementation of the component, even when it would be possible to make the registration work inside an iframe (as explained on this post, for the most general case.

Would be interesting to allow cases like the ones mentioned before work.
@demianrenzulli
Copy link
Contributor Author

cc @andreban

@sparhami
Copy link

It looks like a form of 2 is supported, you can install a serviceworker on the canonicalUrl's origin when the document is hosted on the AMP cache (or rather, the configured CDN URL for the AMP JS). The documentation seems to be explicit about this behavior: https://www.ampproject.org/docs/reference/components/amp-install-serviceworker#behavior. I'm not sure if there is some context of why this is restricted to the AMP cache or if this use case was not considered at the time.

For case 1, I don't think you really need <amp-install-serviceworker>. You could just have an <amp-iframe> which loads an html file that installs the service worker for the subdomain (though, admittedly this would be hacky since you would need to make it really tiny (I think it may need to be larger than 0x0 to actually be loaded) and give it a placeholder (if it is near the top of the viewport). Given that you could do it, I don't see any reason for <amp-install-serviceworker> to prevent you from doing it (that is, I can't see a reason not to remove the origin check for data-iframe-src entirely, and if the the src origin does not match, always use data-iframe-src, regardless if the document is being served from the AMP cache).

@adelinamart adelinamart added this to the New FRs milestone Sep 20, 2018
@sparhami
Copy link

sparhami commented Oct 1, 2018

So after thinking about this a bit more, I'm not sure we can remove the origin check. Otherwise, this could be used as a way of creating <iframe>s, but getting around the restrictions that <amp-iframe> imposes.

@tomayac
Copy link
Contributor

tomayac commented Feb 8, 2019

Otherwise, this could be used as a way of creating <iframe>s, but getting around the restrictions that <amp-iframe> imposes.

Is there a way for AMP to enforce that the <body> of an iframe is and remains empty? The use case described by @demianrenzulli seems legit.

@tomayac
Copy link
Contributor

tomayac commented Feb 11, 2019

My use case would be to use microsites on different domains to promote full PWAs and pre-warm their caches.

@prateekbh
Copy link
Member

@tomayac do you want these microsites to not host their own service worker?

@sparhami
Copy link

sparhami commented Feb 11, 2019 •
edited

My use case would be to use microsites on different domains to promote full PWAs and pre-warm their caches.

If you have no other need for a tracking iframe (using <amp-iframe>) on the microsite, you could use <amp-iframe> to install the service worker by loading an iframe from the PWA domain that installs the service worker.

Perhaps we could count this as a tracking iframe as far as this piece of code is concerned:

amphtml/extensions/amp-iframe/0.1/amp-iframe.js

Lines 363 to 370 in 389ff3b

trackingIframeCount++;
if (trackingIframeCount > 1) {
console/*OK*/.error('Only 1 analytics/tracking iframe allowed per ' +
'page. Please use amp-analytics instead or file a GitHub issue ' +
'for your use case: ' +
'https://github.com/ampproject/amphtml/issues/new');
return Promise.resolve();
}

So you would get either a tracking <amp-iframe> or a cross-domain service worker, but not both.

@tomayac
Copy link
Contributor

tomayac commented Feb 12, 2019

@sparhami One issue might be a potential race condition: if the service worker installation has not happened within the 5s window, then the approach with <amp-iframe> would not work.

@prateekbh No, the microsite would not need its own service worker.

For clarification, I have implemented the microsite approach in this repo, once in regular HTML, once in AMP HTML. The README file documents the AMP issue.

@nainar nainar added this to Triage in PWA and Shadow AMP Aug 5, 2019
@nainar nainar moved this from Triage to To do in PWA and Shadow AMP Aug 5, 2019
@prateekbh prateekbh assigned jridgewell and unassigned prateekbh Aug 6, 2019
@prateekbh
Copy link
Member

adding @jridgewell (owner of amp-install-serviceworker) to be re-assigned

@AndrewKGuan
Copy link

Hi all, any update on this ticket?

@jridgewell
Copy link
Contributor

/cc @ampproject/performance

@stale
Copy link

stale bot commented Mar 12, 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 Mar 12, 2021
@stale stale bot closed this as completed Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jridgewell jridgewell

Labels
Category: PWA Stale Inactive for one year or more Type: Feature Request
Projects
PWA and Shadow AMP
To do (SW)
Milestone
New FRs
Development

No branches or pull requests
8 participants
@jridgewell @tomayac @sparhami @prateekbh @demianrenzulli @ericlindley-g @adelinamart @AndrewKGuan