fix: review comments

Author: SkArchon

composition/src/v1/warnings/params.ts

@@ -16,3 +16,9 @@ export type InvalidRepeatedComposedDirectiveWarningParams = {
   directiveName: DirectiveName;
   printedDirective: string;
 };
+
+export type RequestScopedSingleFieldWarningParams = {
+  subgraphName: SubgraphName;
+  key: string;
+  fieldCoords: string;
+};

composition/src/v1/warnings/warnings.ts

@@ -2,6 +2,7 @@ import { Warning } from '../../warnings/types';
 import { QUOTATION_JOIN } from '../../utils/string-constants';
 import {
   type InvalidRepeatedComposedDirectiveWarningParams,
+  type RequestScopedSingleFieldWarningParams,
   type SingleFederatedInputFieldOneOfWarningParams,
   type SingleSubgraphInputFieldOneOfWarningParams,
 } from './params';
@@ -192,6 +193,22 @@ export function singleSubgraphInputFieldOneOfWarning({
   });
 }
 
+export function requestScopedSingleFieldWarning({
+  subgraphName,
+  key,
+  fieldCoords,
+}: RequestScopedSingleFieldWarningParams): Warning {
+  return new Warning({
+    message:
+      `@openfed__requestScoped(key: "${key}") is declared on only one field ("${fieldCoords}") in this subgraph.` +
+      ` The directive is meaningless unless at least 2 fields share the same key so that the second and` +
+      ` subsequent fields can be served from the per-request L1 cache populated by the first.`,
+    subgraph: {
+      name: subgraphName,
+    },
+  });
+}
+
 export function singleFederatedInputFieldOneOfWarning({
   fieldName,
   typeName,

composition/tests/v1/directives/request-scoped.test.ts

@@ -1,11 +1,11 @@
 import { describe, expect, test } from 'vitest';
 import {
-  DIRECTIVE_DEFINITION_BY_NAME,
   FIRST_ORDINAL,
   invalidDirectiveError,
   invalidRepeatedDirectiveErrorMessage,
   OPENFED_REQUEST_SCOPED,
   OPENFED_REQUEST_SCOPED_DEFINITION,
+  requestScopedSingleFieldWarning,
   ROUTER_COMPATIBILITY_VERSION_ONE,
   undefinedRequiredArgumentsErrorMessage,
 } from '../../../src';
@@ -71,6 +71,30 @@ describe('@openfed__requestScoped tests', () => {
       expect(config!.entityCaching!.requestScopedConfigurations![0].fieldName).toBe('currentLocale');
       expect(config!.entityCaching!.requestScopedConfigurations![0].l1Key).toBe('subgraph-default-a.locale');
     });
+
+    test('that a key declared on only one field still populates config but emits a warning', () => {
+      const result = normalizeSubgraphSuccess(
+        createSubgraphWithDefaultName(`
+          type Query {
+            currentUser: User @openfed__requestScoped(key: "lonely")
+          }
+          type User @key(fields: "id") {
+            id: ID!
+          }
+        `),
+        ROUTER_COMPATIBILITY_VERSION_ONE,
+      );
+      const config = result.configurationDataByTypeName.get('Query');
+      expect(config!.entityCaching?.requestScopedConfigurations).toHaveLength(1);
+      expect(config!.entityCaching!.requestScopedConfigurations![0].l1Key).toBe('subgraph-default-a.lonely');
+      expect(result.warnings).toStrictEqual([
+        requestScopedSingleFieldWarning({
+          subgraphName: 'subgraph-default-a',
+          key: 'lonely',
+          fieldCoords: 'Query.currentUser',
+        }),
+      ]);
+    });
   });
 
   describe('validation tests', () => {

shared/src/router-config/builder.ts

@@ -80,7 +80,9 @@ function costsToCostConfiguration(costs?: Costs): CostConfiguration | undefined
  * @returns The `EntityCaching` message, or `undefined` when empty so the field is omitted (like
  * `costsToCostConfiguration`).
  */
-function extractEntityCachingConfiguration(dataByTypeName?: Map<TypeName, ConfigurationData>): EntityCachingConfiguration | undefined {
+function extractEntityCachingConfiguration(
+  dataByTypeName?: Map<TypeName, ConfigurationData>,
+): EntityCachingConfiguration | undefined {
   if (!dataByTypeName) {
     return;
   }
@@ -134,8 +136,13 @@ function extractEntityCachingConfiguration(dataByTypeName?: Map<TypeName, Config
       );
     }
   }
-  if (entityCache.length > 0 || cacheInvalidateConfigurations.length > 0 || cachePopulateConfigurations.length > 0 || requestScopedConfigurations.length > 0) {
-     return new EntityCachingConfiguration({
+  if (
+    entityCache.length > 0 ||
+    cacheInvalidateConfigurations.length > 0 ||
+    cachePopulateConfigurations.length > 0 ||
+    requestScopedConfigurations.length > 0
+  ) {
+    return new EntityCachingConfiguration({
       entityCache,
       cacheInvalidateConfigurations,
       cachePopulateConfigurations,