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

feat: change the order of default runtimeCache #106

Merged
merged 1 commit into from
Dec 10, 2018

Conversation

gyarasu
Copy link
Contributor

@gyarasu gyarasu commented Nov 30, 2018

Overview

By default, runtimeCache is added for base URL as below.

workbox.routing.registerRoute(new RegExp('/.*'), workbox.strategies.networkFirst({}), 'GET')

Then, I have an problem.
When I add runtimeCache for /api/xxxxxx and set the strategy except for networkFirst, for example cacheFirst, it doesn't work because of the order.
I want to change the default runtimeCache order to avoid this problem.

example setting

  workbox: {
    skipWaiting: true,
    clientsClaim: true,
    runtimeCaching: [
      {
        urlPattern: '/api/xxxxxx/.*',
        handler: 'cacheFirst',
        method: 'GET',
        strategyOptions: {
          cacheExpiration: {
            maxAgeSeconds: 60 * 15, // 15min
          },
          cacheableResponse: {
            statuses: [200],
          },
        },
      },\
    ],
  },

example output

workbox.routing.registerRoute(new RegExp('/.*'), workbox.strategies.networkFirst({}), 'GET')

workbox.routing.registerRoute(new RegExp('/api/xxxxxx/.*'), workbox.strategies. cacheFirst({"cacheExpiration":{"maxAgeSeconds":900},"cacheableResponse":{"statuses":[200]}}), 'GET')

I intend that the request for/api/xxxxxx/.* refers to cache firstly.
But it fetches the data from network firstly.

Solution

/api/xxxxxx matches with both url pattern. Then the first pattern is adopted on workbox.
I think the default runtimeCache setting should be set to the last by default.

Current Work Around

I have an idea to avoid that situation but it is somewhat complex.

  • set false to offlineoption
  • set default url pattern at the end of runtimeCache option

example setting

  workbox: {
    offline: false,
    skipWaiting: true,
    clientsClaim: true,
    runtimeCaching: [
      {
        urlPattern: '/api/xxxxxx/.*',
        handler: 'cacheFirst',
        method: 'GET',
        strategyOptions: {
          cacheExpiration: {
            maxAgeSeconds: 60 * 15, // 15min
          },
          cacheableResponse: {
            statuses: [200],
          },
        },
      },
      {
         urlPattern: '/*',
         handler: 'networkFirst',
         method: 'GET',
       },
    ],
  },

example output

workbox.routing.registerRoute(new RegExp('/api/xxxxxx/.*'), workbox.strategies. cacheFirst({"cacheExpiration":{"maxAgeSeconds":900},"cacheableResponse":{"statuses":[200]}}), 'GET')

workbox.routing.registerRoute(new RegExp('/.*'), workbox.strategies.networkFirst({}), 'GET')

It works.

@codecov
Copy link

codecov bot commented Nov 30, 2018

Codecov Report

Merging #106 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@         Coverage Diff         @@
##            dev   #106   +/-   ##
===================================
  Coverage   100%   100%           
===================================
  Files         1      1           
  Lines         2      2           
===================================
  Hits          2      2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f27d5c...69b7490. Read the comment docs.

Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

Thanks @gyarasu for investigations and fix

@pi0 pi0 merged commit 033b504 into nuxt-community:dev Dec 10, 2018
@gyarasu
Copy link
Contributor Author

gyarasu commented Dec 10, 2018

@pi0
Thanks for your review:)

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

Successfully merging this pull request may close these issues.

2 participants