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

Service Worker route handling refactor #1859

Merged
merged 28 commits into from
Oct 15, 2019

Conversation

revanth0212
Copy link
Contributor

@revanth0212 revanth0212 commented Oct 7, 2019

Description

This PR rearranges Service Worker routes to match that of UPWARD. For now, it does not produce any significant changes but this needed to fix other issues. Also, this adds a new route that matches robots.txt, manifest.json and favicon.ico. So you should be seeing them being served from the service worker expect for the first time. The service worker uses the stale while revalidate strategy to serve these files, meaning it will return what it has immediately to the UI and then fetches the files in the background and updates them in the cache for later use.

Related Issue

Closes #1857

Verification Steps

favicon should be cached on the client. This should be verifiable by looking at the network tab or the caches tab in chrome dev tools.

It is not working as of now in stage mode and a bug has been logged #1861.

Checklist

Just a casual sanity check should be enough.

@revanth0212 revanth0212 added the version: Minor This changeset includes functionality added in a backwards compatible manner. label Oct 7, 2019
@revanth0212
Copy link
Contributor Author

Do not worry about the number of changes, that is because this PR is based on changes that are yet to be merged as part of #1832

@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Oct 7, 2019

Messages
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).

Generated by 🚫 dangerJS against 0529754


workbox.routing.registerRoute(
new RegExp('/.\\.js$'),
new RegExp('(robots.txt|favicon.ico|manifest.json)'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick regex gotcha: that . actually matches on any character. If you want the literal ., it needs to be escaped: \..

Doesn't prevent a 👍 , just an fyi :)

@dpatil-magento dpatil-magento merged commit 5ab658a into develop Oct 15, 2019
@dpatil-magento dpatil-magento deleted the revanth/serviceWorkerRouteHandlingRefactor branch October 15, 2019 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:pwa-buildpack pkg:venia-concept version: Minor This changeset includes functionality added in a backwards compatible manner.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature]: Mirror UPWARD route handling in the Service Worker.
4 participants