Skip to content

Support caching opaque responses with freshness strategy in ngsw-config.json [PWA][SW] #30968

Closed
@borjapazr

Description

@borjapazr

Caching opaque responses with freshness strategy in ngsw-config.json [PWA][SW]

Relevant Package

This feature request is for @angular/pwa.

Description

The SW might have an option to cache opaque responses in the freshness mode since these caches are short-lived and therefore having an inadequate response would be quickly updated/removed.

I was reading ngsw-worker.js's code and now I have one question.
In the next function, what is the purpose of okToCacheOpaque? Is it to avoid having the opaque answers cached or just to the opposite? What is the real usage of this variable now?
Could this variable be used to cache opaque responses?

/**
         * Operation for caching the response from the server. This has to happen all
         * at once, so that the cache and LRU tracking remain in sync. If the network request
         * completes before the timeout, this logic will be run inline with the response flow.
         * If the request times out on the server, an error will be returned but the real network
         * request will still be running in the background, to be cached when it completes.
         */
        cacheResponse(req, res, lru, okToCacheOpaque = false) {
            return __awaiter$1(this, void 0, void 0, function* () {
                // Only cache successful responses.
                if (!res.ok || (okToCacheOpaque && res.type === 'opaque')) {
                    return;
                }
                // If caching this response would make the cache exceed its maximum size, evict something
                // first.
                if (lru.size >= this.config.maxSize) {
                    // The cache is too big, evict something.
                    const evictedUrl = lru.pop();
                    if (evictedUrl !== null) {
                        yield this.clearCacheForUrl(evictedUrl);
                    }
                }
                // TODO: evaluate for possible race conditions during flaky network periods.
                // Mark this resource as having been accessed recently. This ensures it won't be evicted
                // until enough other resources are requested that it falls off the end of the LRU chain.
                lru.accessed(req.url);
                // Store the response in the cache (cloning because the browser will consume
                // the body during the caching operation).
                yield (yield this.cache).put(req, res.clone());
                // Store the age of the cache.
                const ageTable = yield this.ageTable;
                yield ageTable.write(req.url, { age: this.adapter.time });
                // Sync the LRU chain to non-volatile storage.
                yield this.syncLru();
            });
        }

Describe the solution you'd like

Caching opaque responses in the freshness mode of dataGroups in ngsw-config.json

Describe alternatives you've considered

https://medium.com/@sascha.wolff/how-to-use-angular-6-service-worker-for-dynamically-loading-images-f4f6f497699f

Activity

changed the title [-]Support caching of opaque responses with freshness strategy in ngsw-config.json [PWA][SW][/-] [+]Support caching opaque responses with freshness strategy in ngsw-config.json [PWA][SW][/+] on Jun 11, 2019
added this to the needsTriage milestone on Jun 11, 2019
gkalpak

gkalpak commented on Jun 11, 2019

@gkalpak
Member

You are right, @borjapazr. It looks like a bug in the if condition (since okToCacheOpaque is never taken into account atm, because opaque responses have res.ok === false). I assume the intention was to negate the condition:

-if (!res.ok || (okToCacheOpaque && res.type === 'opaque')) {
+if (!(res.ok || (okToCacheOpaque && res.type === 'opaque'))) {

I have a fix for that in #30977 (see 7352a7f), but we should also add some docs about this (e.g. similar to workbox's).

alxhub

alxhub commented on Jun 11, 2019

@alxhub
Member

@gkalpak that change looks right to me.

borjapazr

borjapazr commented on Jun 11, 2019

@borjapazr
ContributorAuthor

You are right, @borjapazr. It looks like a bug in the if condition (since okToCacheOpaque is never taken into account atm, because opaque responses have res.ok === false). I assume the intention was to negate the condition:

-if (!res.ok || (okToCacheOpaque && res.type === 'opaque')) {
+if (!(res.ok || (okToCacheOpaque && res.type === 'opaque'))) {

I have a fix for that in #30977 (see 7352a7f), but we should also add some docs about this (e.g. similar to workbox's).

Now it makes sense to me. Thank you @gkalpak.

And adding more documentation would be a great idea 😊

added a commit that references this issue on Jun 27, 2019
b0c3453
angular-automatic-lock-bot

angular-automatic-lock-bot commented on Sep 15, 2019

@angular-automatic-lock-bot

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

1 remaining item

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Participants

      @alxhub@gkalpak@borjapazr

      Issue actions

        Support caching opaque responses with freshness strategy in ngsw-config.json [PWA][SW] · Issue #30968 · angular/angular