@graphql-yoga/plugin-response-cache wants to build cache keys for requests where enabled(request) === false

[klippx]

In the source code of @graphql-yoga/plugin-response-cache, the user defined enabled(request) is invoked after user defined fn buildResponseCacheKey which is confusing. Let's say for example this is an introspection query or it should be skipped based on headers, then that seems to be established. However, then suddenly you are surprised by the fact that you still have to deal with the fact that buildResponseCacheKey is invoked to build a cache key anyway, even though it was already "determined" it was disabled since it was an introspection query / skipped request!

This is very confusing for me and our users, it seems like you want to check enabled(request) first and THEN invoke buildResponseCacheKey because why would you want to build a cache key for something that should not be cached anyway?

        async onParams({ params, request, setResult }) {
            const sessionId = await options.session(request);
            const operationId = await buildResponseCacheKey({
                documentString: params.query || '',
                variableValues: params.variables,
                operationName: params.operationName,
                sessionId,
                request,
            });
            operationIdByRequest.set(request, operationId);
            sessionByRequest.set(request, sessionId);
            if (enabled(request)) {
                const cachedResponse = await cache.get(operationId);
                if (cachedResponse) {
                    if (options.includeExtensionMetadata) {
                        setResult(resultWithMetadata(cachedResponse, { hit: true }));
                    }
                    else {
                        setResult(cachedResponse);
                    }
                    return;
                }
            }
        },

@EmrysMyrddin

[EmrysMyrddin]

Yes, I noticed this too last week. I have to check why it has been done this way, because in the past, the cache was calculated only if cache is enabled for the request. I have to dig a little more to understand the reason for this change.

[klippx]

I opened an issue #3034