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

sw-precaching and Routing #136

Closed
gauntface opened this issue Jan 13, 2017 · 6 comments
Closed

sw-precaching and Routing #136

gauntface opened this issue Jan 13, 2017 · 6 comments
Assignees

Comments

@gauntface
Copy link

sw-precaching at the moment ONLY caches assets, it doesn't define any routes.

There are a few things to consider:

  1. If it sets up the routes inside the module, that blocks off any possibility of external routes doing the caching.
  2. If we want external routes to be defined to do this, there is no exposure of the cache name and be default I think caching strategies look in a custom cache name.

Solutions:

  1. Add the routes in sw-precaching.
  2. Add the routes in sw-precaching with opt out in options.
  3. Add a way for routes to match against any cache (i.e. caches.matches() )
  4. Add a way to get the cache name from sw-precaching and configure route for that.
@jeffposnick
Copy link
Contributor

jeffposnick commented Jan 13, 2017

How about this? (I think it's pretty similar to what was in a design doc somewhere.)

const precacheManager = new goog.precaching.PrecacheManager();
// Do stuff with precacheManager to set up the manifest, etc....

const router = new goog.routing.Router();

// This would create a Route that knew exactly which URLs to match and would use a
// cache-first strategy as the handler.
const precacheRoute = precacheManager.createRoute();
router.registerRoute({route: precacheRoute});

// Register any additional runtime routes at this point.

That's the lower-level picture, of course. Maybe there's an opportunity to have sw-lib paper over some of the details?

@gauntface
Copy link
Author

That sounds like a good mix of giving the developer choice when using the module directly and sw-lib can super easily make this so.

Will make a PR for this to see what you think of an approach.

@jeffposnick
Copy link
Contributor

Here's another wrinkle: sw-precache currently supports fallback-to-App Shell-for-navigation-request style routing via a combination of the navigateFallback and navigateFallbackWhitelist options.

Routing navigation requests to the cached App Shell is an important use case, so we should address it somehow:

  1. The proposed precacheManager.createRoute() method can take in additional parameters equivalent to navigateFallback/navigateFallbackWhitelist (but hopefully named something better). Then the Route it creates could handle navigation requests appropriately. This presupposes that everyone using the App Shell architecture will also be using PrecacheManager.

  2. Create a new AppShellRoute subclass of Route that behaved much the same way, but wasn't as tied to PrecacheManager. This would be more flexible, but it means more things for a developer to configure and/or forget to include. Some of that additional developer pain might be smoothed over by a sw-lib abstraction, though.

  3. Something else?

If we go with 1., it would probably fall to @gauntface as part of the related work tracked in this issue. If we go with 2., I'll file a separate issue and assign it to myself.

@gauntface gauntface self-assigned this Jan 19, 2017
@gauntface
Copy link
Author

gauntface commented Jan 19, 2017

Thinking on this a little more and struggling to picture the best way to do it with the current API.

Discussing the navigateFallback in relation to precache management is a bit of a wake up call that this might not be the best approach.

An alternative approach to the above is to just not include routing within precaching and instead expose the required details so a Route could be constructed. Developers could use the cacheName to check the specific cache and the cached urls could be exposed for the match method.

Basically we'd up with something along these lines (Ignoring the URL parsing / dance to make them actually the same format):

precacheManager.cacheRevisionedAssets([........]);
const route = new Route({
    match: ({url, event} => {
      const cachedUrls = precacheManager.getCachedUrls();
      if (cachedUrls.indexOf(url) !== -1) {
        return true;
      }
      return false;
    }),
    handler: new CacheFirstHandler({
        cacheName: precacheManager.getCacheName()
    })
});

This approach could be abstracted to a separate Route that can wrap everything up nicely (useful for consumers of the small libraries). With or without a "PrecacheRoute", sw-lib can abstract this away from consumers of the higher level library.

This approach would then also leave the navigateFallback to a seperate discussion as to how it's handled by the Router.

@jeffposnick
Copy link
Contributor

Hiding magic and papering over the low-level libraries details—like the cache name and list of resources that PrecacheManager could expose—seems to be the raison d'être for SWLib, so I'm 👍 with your proposal. I don't know that we'd want to create a PrecacheRoute in that case, since the base Route class is sufficient to handle what we want.

We could move the navigateFallback discussion to a different issue.

@gauntface
Copy link
Author

coolio and 👍 to moving navigateFallback to a seperate issue to track it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants