feat(response-cache): add extras property to get scope and its metadata from buildResponseCacheKey #2202

k-i-k-s · 2024-04-02T16:15:33Z

Issue related : #2181

Description

Include a new extras property to get scope (as a Proxy to avoid performance impact when unused) in buildResponseCacheKey, allowing more granularity over cache control.

Type of change

New feature (non-breaking change which adds functionality)
This change requires a documentation update

How Has This Been Tested?

I added some unit tests to check that PRIVATE scope is correctly retrieved from a query, when directive @cacheControl(scope: PRIVATE) is put on query, field and subfield.

Checklist:

I have followed the
CONTRIBUTING doc and the
style guidelines of this project
I have performed a self-review of my own code
I have commented on my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

changeset-bot · 2024-04-02T16:15:37Z

🦋 Changeset detected

Latest commit: 9e8ea70

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@envelop/response-cache	Minor
@envelop/response-cache-cloudflare-kv	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

…ta from buildResponseCacheKey

k-i-k-s · 2025-10-29T15:27:58Z

Hi @ardatan, I'm back with a new proposal for this scope functionality with some improvements.
Tell me what you think about those changes!

ardatan · 2025-10-29T17:11:42Z

@EmrysMyrddin Could you take a look?

EmrysMyrddin · 2025-10-29T19:30:54Z

packages/plugins/response-cache/src/plugin.ts

+            new Proxy(
+              {
+                documentString: getDocumentString(onExecuteParams.args),
+                variableValues: onExecuteParams.args.variableValues,
+                operationName: onExecuteParams.args.operationName,
+                sessionId,
+                context: onExecuteParams.args.contextValue,
+                extras: undefined as any,
+              },
+              {
+                get: (obj, prop) => {
+                  if (prop === 'extras') {
+                    return getScopeFromQuery(schema, onExecuteParams.args.document.loc.source.body);
+                  }
+                  return obj[prop as keyof typeof obj];
+                },
+              },
+            ),


I don't remembre how it was implemented before, but is there a reason to use a proxy instead of classic getters?

Like this:

Suggested change

new Proxy(

{

documentString: getDocumentString(onExecuteParams.args),

variableValues: onExecuteParams.args.variableValues,

operationName: onExecuteParams.args.operationName,

sessionId,

context: onExecuteParams.args.contextValue,

extras: undefined as any,

},

{

get: (obj, prop) => {

if (prop === 'extras') {

return getScopeFromQuery(schema, onExecuteParams.args.document.loc.source.body);

}

return obj[prop as keyof typeof obj];

},

},

),

{

documentString: getDocumentString(onExecuteParams.args),

variableValues: onExecuteParams.args.variableValues,

operationName: onExecuteParams.args.operationName,

sessionId,

context: onExecuteParams.args.contextValue,

get extras() {

return getScopeFromQuery(schema, onExecuteParams.args.document.loc.source.body);

},

},

A recent habit to use proxies for their flexibility, but I can see that a getter in this case would fit better for clarity sake.

EmrysMyrddin · 2025-10-29T19:33:29Z

packages/plugins/response-cache/src/get-scope.ts

+    return { scope: 'PUBLIC' as const, metadata: {} };
+  });
+
+  return fn({ query });


This will disallow memoization isn't it ? Since { query } creates a new object each time the function is called.

And I'm not sure to understand why schema is not part of the memoize1 key ? Because I fear that on schema update, you will get outadated metadata for the same query.

Fair points!
I'll include schema using memoize2 function and stringify those objects to fix comparison.

EmrysMyrddin · 2025-10-29T19:41:44Z

packages/plugins/response-cache/src/plugin.ts

+export let schema: GraphQLSchema;
+let ttlPerSchemaCoordinate: Record<string, CacheControlDirective['maxAge']> = {};
+let scopePerSchemaCoordinate: Record<string, CacheControlDirective['scope']> = {};


Why do you extract those outside of the plugin? I feel that it will break things, because multiple instances of the same plugin would share those variables.

EmrysMyrddin · 2025-10-29T19:56:58Z

packages/plugins/response-cache/src/plugin.ts

+export function isPrivate(
+  typeName: string,
+  data?: Record<string, NonNullable<CacheControlDirective['scope']>>,
+): boolean {
+  if (scopePerSchemaCoordinate[typeName] === 'PRIVATE') {
+    return true;
+  }
+  return data
+    ? Object.keys(data).some(
+        fieldName => scopePerSchemaCoordinate[`${typeName}.${fieldName}`] === 'PRIVATE',
+      )
+    : false;
+}


Why do you have to make a duplicate of this function outside of the plugin?

Mb I merged local code I made last year with the one I use today and simply forgot there was already this function.
I'll keep the exported one since it's needed in get-scope.ts file.

… better scope memoization

k-i-k-s · 2025-10-30T11:18:53Z

Hi @EmrysMyrddin, how's it going today ?

I made updates to fix the points you brought up :

extras is a function now and the schema is its parameter
no more exported schema since it's a param of extras
memoization fixed with a combination of WeakMap (schema as the key) containing an LRU cache per schema (using query as key)
also added hitCache boolean property in metadata and updated tests accordingly

k-i-k-s force-pushed the feat/response-cache-query-scope branch 6 times, most recently from 4f30aba to a5468bb Compare April 3, 2024 16:50

k-i-k-s force-pushed the feat/response-cache-query-scope branch from a5468bb to 9e8ea70 Compare April 17, 2024 13:32

feat(response-cache): add extras property to get scope and its metada…

c4de9af

…ta from buildResponseCacheKey

k-i-k-s force-pushed the feat/response-cache-query-scope branch from 9e8ea70 to c4de9af Compare October 29, 2025 14:49

k-i-k-s changed the title ~~feat(response-cache): added getScope callback in buildResponseCacheKey~~ feat(response-cache): add extras property to get scope and its metadata from buildResponseCacheKey Oct 29, 2025

ardatan requested a review from EmrysMyrddin October 29, 2025 17:11

EmrysMyrddin reviewed Oct 29, 2025

View reviewed changes

feat(response-cache): extras as a function with schema as a param and…

d4ab39b

… better scope memoization

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(response-cache): add extras property to get scope and its metadata from buildResponseCacheKey #2202

feat(response-cache): add extras property to get scope and its metadata from buildResponseCacheKey #2202

Uh oh!

k-i-k-s commented Apr 2, 2024 •

edited

Loading

Uh oh!

changeset-bot bot commented Apr 2, 2024 •

edited

Loading

Uh oh!

k-i-k-s commented Oct 29, 2025

Uh oh!

ardatan commented Oct 29, 2025

Uh oh!

EmrysMyrddin Oct 29, 2025 •

edited

Loading

Uh oh!

k-i-k-s Oct 29, 2025

Uh oh!

EmrysMyrddin Oct 29, 2025

Uh oh!

EmrysMyrddin Oct 29, 2025

Uh oh!

k-i-k-s Oct 29, 2025

Uh oh!

EmrysMyrddin Oct 29, 2025

Uh oh!

EmrysMyrddin Oct 29, 2025

Uh oh!

k-i-k-s Oct 29, 2025

Uh oh!

k-i-k-s commented Oct 30, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

feat(response-cache): add extras property to get scope and its metadata from buildResponseCacheKey #2202

Are you sure you want to change the base?

feat(response-cache): add extras property to get scope and its metadata from buildResponseCacheKey #2202

Uh oh!

Conversation

k-i-k-s commented Apr 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How Has This Been Tested?

Checklist:

Uh oh!

changeset-bot bot commented Apr 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

k-i-k-s commented Oct 29, 2025

Uh oh!

ardatan commented Oct 29, 2025

Uh oh!

EmrysMyrddin Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k-i-k-s Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

EmrysMyrddin Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

EmrysMyrddin Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

k-i-k-s Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

EmrysMyrddin Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

EmrysMyrddin Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

k-i-k-s Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

k-i-k-s commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

k-i-k-s commented Apr 2, 2024 •

edited

Loading

changeset-bot bot commented Apr 2, 2024 •

edited

Loading

EmrysMyrddin Oct 29, 2025 •

edited

Loading

k-i-k-s commented Oct 30, 2025 •

edited

Loading