From a33a530e0ee1e507daf6ddf16298ace87238d61b Mon Sep 17 00:00:00 2001 From: Sam Curry Date: Mon, 17 Aug 2026 12:07:42 +0800 Subject: [PATCH 1/5] feat(deprecation): collect deprecated schema element usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `collectDeprecatedElementUsage`, a pure collector reporting every `@deprecated` element an operation uses, so teams can tell whether a deprecated element is safe to remove. Output fields, arguments and directive arguments are named in the document, so an AST walk finds them. Input fields and enum values are data the caller supplies rather than selects, and can arrive either as a document literal or inside a variable where the name appears nowhere in the AST, so the supplied variable values are walked too, against their declared input types. Ported from an app-local Apollo plugin. Fixed on the way in: - The AST pass walked every operation in the document, reporting elements from operations that never ran, and fragments only those operations could reach. Now scoped via `separateOperations`. - Directive arguments were attributed to the enclosing field, naming a field argument that does not exist. TypeInfo resolves an argument against the enclosing directive when there is one, so these are now reported as `directive-argument`. - Only input fields and enum values were deduplicated, so an aliased document could emit one record per occurrence. All kinds now dedupe on kind and name, and results are sorted. - The walk was bounded by depth but not breadth or result size, adding `maxVariableNodes` and `maxElements`. - `path` is now best-effort and documented as such: it is fragment-relative for elements inside a fragment definition, so it is a debugging aid rather than an aggregation key. Consumers get the result via `deprecatedElements` on the operation log entry, which is omitted when empty, and via `includeDeprecatedElements` on `useSubscriptionsServer` — collected when the subscription is established rather than per emitted payload. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 32 ++ package.json | 2 +- src/deprecation.spec.ts | 652 ++++++++++++++++++++++++++++++++++++ src/deprecation.ts | 317 ++++++++++++++++++ src/index.ts | 1 + src/logging.spec.ts | 46 +++ src/logging.ts | 26 +- src/subscriptions/server.ts | 14 +- 8 files changed, 1087 insertions(+), 3 deletions(-) create mode 100644 src/deprecation.spec.ts create mode 100644 src/deprecation.ts create mode 100644 src/logging.spec.ts diff --git a/README.md b/README.md index 02a402d..aa8f95c 100644 --- a/README.md +++ b/README.md @@ -281,6 +281,36 @@ Refer to the `GraphQLLogOperationInfo` type for the definition of input. This function can be used across implementations, e.g. in a [GraphQL Envelop plugin](https://www.envelop.dev/docs/plugins) or [ApolloServer plugin](https://github.com/MakerXStudio/graphql-apollo-server/blob/main/src/plugins/graphql-operation-logging-plugin.ts). +## Deprecation usage + +### collectDeprecatedElementUsage + +Reports every `@deprecated` schema element an operation uses, so you can tell whether a deprecated element is safe to remove. + +```ts +const deprecatedElements = collectDeprecatedElementUsage({ schema, document, operation, operationName, variables }) +// [{ kind: 'input-field', name: 'WidgetFilterInput.legacyId', deprecationReason: 'Use id.', path: '$input.legacyId' }] +``` + +Five kinds are detected: `output-field`, `argument`, `directive-argument`, `input-field` and `enum-value`. + +Output fields and arguments are named in the operation document, so an AST walk finds them. Input fields and enum values are data the caller _supplies_ rather than selects, and can arrive either as a document literal or inside a variable — where the name appears nowhere in the AST — so the supplied variable values are walked too, against their declared input types. + +Consumers generally don't call this directly. It is wired into `useSubscriptionsServer` via `includeDeprecatedElements` (below), and into Apollo Server by the `includeDeprecatedElements` option of [`graphqlOperationLoggingPlugin`](https://github.com/MakerXStudio/graphql-apollo-server), which adds the result to the operation log entry as `deprecatedElements`. + +Behaviour worth knowing: + +- **Results are deduplicated** on kind and name, and sorted. One request produces at most one entry per distinct element, so a 40-element list carrying the same deprecated field yields one record, not 40. +- **Only the executed operation counts.** A document may hold several operations but only one runs, so the others — and any fragments only they reach — are excluded. +- **Presence is the signal, explicit `null` included.** A client still sending a field would break if it were removed. +- **It never throws on malformed variables.** The walk runs before graphql-js coerces variables, so a payload whose shape contradicts its declared type reaches it; such values yield no records rather than an error. +- **Work is bounded** by `maxVariableDepth` (default 25), `maxVariableNodes` (default 10,000) and `maxElements` (default 50). A result of exactly `maxElements` should be treated as possibly incomplete. + +Two limitations to design around: + +- **This is request-side telemetry.** It observes values a caller _sends_, never values the server returns. A deprecated enum value that only ever appears in responses reads as unused however often the server returns it. +- **`path` is a best-effort debugging aid, not an aggregation key.** Aggregate on `name`. A field selected inside a fragment definition has no enclosing field in its ancestry, so its path is relative to the fragment rather than the operation — which is the common case for clients that use generated fragments. + ## GraphQL subscriptions This library includes a `subscriptions` module to provide simple setup using the [GraphQL WS](https://the-guild.dev/graphql/ws) package. @@ -335,6 +365,8 @@ This library includes a `subscriptions` module to provide simple setup using the - GraphQL context creation - Logging from the server `onConnect`, `onDisconnect`, `onOperation`, `onNext` and `onError` callbacks + Pass `includeDeprecatedElements: true` to add [deprecation usage](#collectdeprecatedelementusage) to the operation log entry as `deprecatedElements`. It is collected once, when the subscription is established, rather than per emitted payload — the usage is a property of the operation, not of each event. + Example for Apollo Server (`wsServerCleanup` called in the `drainServer` plugin callback): ```ts diff --git a/package.json b/package.json index 9f150fb..f42b8af 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@makerx/graphql-core", - "version": "3.2.0", + "version": "3.3.0", "private": false, "description": "A set of core GraphQL utilities that MakerX uses to build GraphQL APIs", "author": "MakerX", diff --git a/src/deprecation.spec.ts b/src/deprecation.spec.ts new file mode 100644 index 0000000..ffb6207 --- /dev/null +++ b/src/deprecation.spec.ts @@ -0,0 +1,652 @@ +import { buildSchema, parse } from 'graphql' +import { describe, expect, it } from 'vitest' +import type { CollectDeprecatedElementUsageOptions, DeprecatedElementKind, DeprecatedElementUsage } from './deprecation' +import { collectDeprecatedElementUsage } from './deprecation' + +/** + * A schema owned by these tests. Every deprecation in a real schema exists to be removed, so + * pointing this suite at one would make it fail the moment someone completes a migration it has + * nothing to do with. These fixtures are permanent by construction. + * + * Deprecated fixtures, one per case the collector has to handle: + * - `Widget.legacyName`: output field + * - `Query.widget(legacyId:)`: field argument + * - `@audit(legacyTag:)`: directive argument + * - `WidgetFilterInput.legacyId`: input field, top level + * - `WidgetFilterInput.legacyNested`: input field whose own type is an input object + * - `NestedInput.legacyFlag`: input field one level down + * - `TagInput.legacyLabel`: input field inside a list element + * - `WidgetStatus.LEGACY_STATUS`: enum value, reachable as an argument, as an input field + * (`WidgetFilterInput.status`) and inside a list (`WidgetFilterInput.statuses`) + */ +const typeDefs = /* GraphQL */ ` + directive @audit(tag: String, legacyTag: String @deprecated(reason: "Use tag.")) on FIELD + + type Query { + widget(id: ID, legacyId: ID @deprecated(reason: "Use id."), status: WidgetStatus): Widget + } + + type Mutation { + saveWidget(input: WidgetFilterInput!): Widget + archiveWidget(input: ArchiveWidgetInput!): Widget + } + + type Widget { + id: ID + name: String + legacyName: String @deprecated(reason: "Use name.") + } + + enum WidgetStatus { + ACTIVE + LEGACY_STATUS @deprecated(reason: "Use ACTIVE.") + } + + input WidgetFilterInput { + id: ID + legacyId: ID @deprecated(reason: "Use id.") + nested: NestedInput + legacyNested: NestedInput @deprecated(reason: "Use nested.") + tags: [TagInput!] + status: WidgetStatus + statuses: [WidgetStatus!] + } + + input NestedInput { + flag: Boolean + legacyFlag: Boolean @deprecated(reason: "Use flag.") + """ + Self-recursive so a test can nest deeper than the walk's depth cap. + """ + child: NestedInput + } + + input TagInput { + label: String + legacyLabel: String @deprecated(reason: "Use label.") + } + + """ + Declares an input variable like saveWidget does, but with no legacyId, so a test can prove + variables are attributed to the executed operation rather than every operation in the document. + """ + input ArchiveWidgetInput { + id: ID + reason: String + } +` + +const schema = buildSchema(typeDefs) + +type CollectOverrides = Omit + +function collect(query: string, overrides: CollectOverrides = {}): DeprecatedElementUsage[] { + return collectDeprecatedElementUsage({ schema, document: parse(query), ...overrides }) +} + +const names = (usages: DeprecatedElementUsage[]): string[] => usages.map((usage) => usage.name) + +const ofKind = (usages: DeprecatedElementUsage[], kind: DeprecatedElementKind): DeprecatedElementUsage[] => + usages.filter((usage) => usage.kind === kind) + +type NestedInput = { flag?: boolean | null; legacyFlag?: boolean | null; child?: NestedInput | null } + +function nestChildren(depth: number): NestedInput { + let node: NestedInput = { legacyFlag: true } + for (let i = 0; i < depth; i++) node = { child: node } + return node +} + +/** Sends whatever `input` it is given as a variable — the shape real clients use. */ +const saveWidgetViaVariable = /* GraphQL */ ` + mutation InputFieldViaVariables($input: WidgetFilterInput!) { + saveWidget(input: $input) { + name + } + } +` + +describe('collectDeprecatedElementUsage', () => { + it('the fixture schema still carries every deprecation these tests rely on', () => { + // Guards against the suite silently going vacuous if a fixture loses its @deprecated directive, + // and confirms buildSchema preserves @deprecated on argument and input field definitions. + const deprecationOf = (typeName: string, fieldName: string): string | null | undefined => { + const type = schema.getType(typeName) + if (type == null || !('getFields' in type)) return undefined + return type.getFields()[fieldName]?.deprecationReason + } + + expect(deprecationOf('Widget', 'legacyName')).toBe('Use name.') + expect(deprecationOf('WidgetFilterInput', 'legacyId')).toBe('Use id.') + expect(deprecationOf('WidgetFilterInput', 'legacyNested')).toBe('Use nested.') + expect(deprecationOf('NestedInput', 'legacyFlag')).toBe('Use flag.') + expect(deprecationOf('TagInput', 'legacyLabel')).toBe('Use label.') + + const widgetField = schema.getQueryType()?.getFields().widget + expect(widgetField?.args.find((arg) => arg.name === 'legacyId')?.deprecationReason).toBe('Use id.') + + expect(schema.getDirective('audit')?.args.find((arg) => arg.name === 'legacyTag')?.deprecationReason).toBe('Use tag.') + + const status = schema.getType('WidgetStatus') + expect(status != null && 'getValue' in status ? status.getValue('LEGACY_STATUS')?.deprecationReason : undefined).toBe('Use ACTIVE.') + + // The replacements must NOT be deprecated, or the negative controls prove nothing. + expect(status != null && 'getValue' in status ? (status.getValue('ACTIVE')?.deprecationReason ?? null) : undefined).toBeNull() + expect(deprecationOf('Widget', 'name') ?? null).toBeNull() + expect(widgetField?.args.find((arg) => arg.name === 'id')?.deprecationReason ?? null).toBeNull() + }) + + describe('elements found in the document', () => { + it('records a deprecated output field', () => { + const usages = collect(/* GraphQL */ ` + query OutputField { + widget(id: "w1") { + legacyName + } + } + `) + + expect(usages).toEqual([ + { kind: 'output-field', name: 'Widget.legacyName', deprecationReason: 'Use name.', path: 'widget.legacyName' }, + ]) + }) + + it('records a deprecated argument given a literal value', () => { + const usages = collect(/* GraphQL */ ` + query ArgumentLiteral { + widget(legacyId: "w1") { + name + } + } + `) + + expect(usages).toEqual([{ kind: 'argument', name: 'Query.widget(legacyId)', deprecationReason: 'Use id.', path: 'widget' }]) + }) + + it('records a deprecated argument even when its value comes from a variable', () => { + // The argument is named in the document (`legacyId: $legacyId`) even though its value is a + // variable — precisely what an input-object field never is. + const usages = collect( + /* GraphQL */ ` + query ArgumentViaVariables($legacyId: ID) { + widget(legacyId: $legacyId) { + name + } + } + `, + { variables: { legacyId: 'w1' } }, + ) + + expect(names(usages)).toEqual(['Query.widget(legacyId)']) + }) + + it('attributes a deprecated directive argument to the directive, not the enclosing field', () => { + // TypeInfo resolves an argument against the enclosing directive when there is one, so without + // an explicit branch this reports `Query.widget(legacyTag)` — an argument that does not exist. + const usages = collect(/* GraphQL */ ` + query DirectiveArgument { + widget(id: "w1") @audit(legacyTag: "x") { + name + } + } + `) + + expect(usages).toEqual([{ kind: 'directive-argument', name: '@audit(legacyTag)', deprecationReason: 'Use tag.', path: 'widget' }]) + }) + + it('records nothing when the operation selects no deprecated element', () => { + const usages = collect(/* GraphQL */ ` + query NoDeprecatedElements { + widget(id: "w1") { + name + } + } + `) + + expect(usages).toEqual([]) + }) + + it('records one entry when the same output field is selected under several aliases', () => { + const usages = collect(/* GraphQL */ ` + query AliasedTwice { + a: widget(id: "w1") { + legacyName + } + b: widget(id: "w2") { + legacyName + } + } + `) + + expect(names(usages)).toEqual(['Widget.legacyName']) + }) + }) + + /** + * Input fields are kept distinct from output fields because the two are different migrations and + * can share a `.` name. Anything querying this telemetry to decide whether a + * deprecated element is safe to remove filters on exactly that value. + */ + describe('deprecated input fields', () => { + it('records an input field sent via variables', () => { + const usages = collect(saveWidgetViaVariable, { variables: { input: { legacyId: 'w1' } } }) + + expect(usages).toEqual([ + { kind: 'input-field', name: 'WidgetFilterInput.legacyId', deprecationReason: 'Use id.', path: '$input.legacyId' }, + ]) + }) + + it('records an input field written inline in the document', () => { + const usages = collect(/* GraphQL */ ` + mutation InputFieldInline { + saveWidget(input: { legacyId: "w1" }) { + name + } + } + `) + + expect(usages).toEqual([ + { kind: 'input-field', name: 'WidgetFilterInput.legacyId', deprecationReason: 'Use id.', path: 'saveWidget.legacyId' }, + ]) + }) + + it('records an input field alongside a deprecated output field on the same request', () => { + const usages = collect( + /* GraphQL */ ` + mutation InputFieldAndOutputField($input: WidgetFilterInput!) { + saveWidget(input: $input) { + legacyName + } + } + `, + { variables: { input: { legacyId: 'w1' } } }, + ) + + // One request, two kinds — each under its own tag, so a field query and an input-field query + // pick up one apiece rather than both landing in the same bucket. + expect(names(ofKind(usages, 'output-field'))).toEqual(['Widget.legacyName']) + expect(names(ofKind(usages, 'input-field'))).toEqual(['WidgetFilterInput.legacyId']) + }) + + it('records an input field explicitly sent as null — removing it would still break the caller', () => { + const usages = collect(saveWidgetViaVariable, { variables: { input: { legacyId: null } } }) + + expect(names(usages)).toEqual(['WidgetFilterInput.legacyId']) + }) + + it('records nothing when the input object omits the deprecated field', () => { + const usages = collect(saveWidgetViaVariable, { variables: { input: { id: 'w1' } } }) + + expect(usages).toEqual([]) + }) + + it('records input fields nested below the variable type', () => { + const usages = collect(saveWidgetViaVariable, { + variables: { input: { nested: { legacyFlag: true }, legacyNested: { flag: true } } }, + }) + + expect(usages).toEqual([ + { kind: 'input-field', name: 'NestedInput.legacyFlag', deprecationReason: 'Use flag.', path: '$input.nested.legacyFlag' }, + { kind: 'input-field', name: 'WidgetFilterInput.legacyNested', deprecationReason: 'Use nested.', path: '$input.legacyNested' }, + ]) + }) + + it('records an input field inside a list element, with its index in the path', () => { + const usages = collect(saveWidgetViaVariable, { + variables: { input: { tags: [{ label: 'kept' }, { legacyLabel: 'gone' }] } }, + }) + + expect(usages).toEqual([ + { kind: 'input-field', name: 'TagInput.legacyLabel', deprecationReason: 'Use label.', path: '$input.tags.1.legacyLabel' }, + ]) + }) + + it('records one entry however many list elements carry the same field', () => { + // Deduplicated per request: without it a 40-element list would emit 40 identical records, + // letting one request flood the audit log. + const usages = collect(saveWidgetViaVariable, { + variables: { input: { tags: Array.from({ length: 40 }, (_, i) => ({ legacyLabel: `tag-${i}` })) } }, + }) + + expect(names(usages)).toEqual(['TagInput.legacyLabel']) + }) + + it('records one entry when the same field arrives via both a variable and a document literal', () => { + const usages = collect( + /* GraphQL */ ` + mutation InputFieldFromBothSources($input: WidgetFilterInput!) { + fromVariable: saveWidget(input: $input) { + name + } + fromLiteral: saveWidget(input: { legacyId: "w1" }) { + name + } + } + `, + { variables: { input: { legacyId: 'w1' } } }, + ) + + expect(names(usages)).toEqual(['WidgetFilterInput.legacyId']) + }) + + it('walks variables against the executed operation, not every operation in the document', () => { + // Both operations declare `$input`, with a different type each. `legacyId` is deprecated on + // WidgetFilterInput and absent from ArchiveWidgetInput, so crediting the unexecuted one lies. + const usages = collect( + /* GraphQL */ ` + mutation SaveWidget($input: WidgetFilterInput!) { + saveWidget(input: $input) { + name + } + } + + mutation ArchiveWidget($input: ArchiveWidgetInput!) { + archiveWidget(input: $input) { + name + } + } + `, + // Deliberately carries a field ArchiveWidgetInput doesn't declare — that is the point. + { variables: { input: { legacyId: 'w1' } }, operationName: 'ArchiveWidget' }, + ) + + expect(usages).toEqual([]) + }) + }) + + describe('operation scoping', () => { + it('ignores literals belonging to an operation that was not executed', () => { + const usages = collect( + /* GraphQL */ ` + query Executed { + widget(id: "w1") { + name + } + } + + query NotExecuted { + widget(legacyId: "w1") { + legacyName + } + } + `, + { operationName: 'Executed' }, + ) + + expect(usages).toEqual([]) + }) + + it('ignores fragments reachable only from an operation that was not executed', () => { + const usages = collect( + /* GraphQL */ ` + query Executed { + widget(id: "w1") { + ...KeptFields + } + } + + query NotExecuted { + widget(id: "w1") { + ...LegacyFields + } + } + + fragment KeptFields on Widget { + name + } + + fragment LegacyFields on Widget { + legacyName + } + `, + { operationName: 'Executed' }, + ) + + expect(usages).toEqual([]) + }) + + it('reports elements reachable through a fragment used by the executed operation', () => { + const usages = collect( + /* GraphQL */ ` + query Executed { + widget(id: "w1") { + ...LegacyFields + } + } + + fragment LegacyFields on Widget { + legacyName + } + `, + { operationName: 'Executed' }, + ) + + expect(names(usages)).toEqual(['Widget.legacyName']) + }) + + it('resolves the only operation in a document when no operation name is given', () => { + const usages = collect(/* GraphQL */ ` + { + widget(id: "w1") { + legacyName + } + } + `) + + expect(names(usages)).toEqual(['Widget.legacyName']) + }) + }) + + /** + * Deprecated enum values reach the server the same two ways an input field does — as a document + * literal, or as their name inside a variable — so both passes look for them. + */ + describe('deprecated enum values', () => { + it('records an enum value written as a literal in an argument', () => { + const usages = collect(/* GraphQL */ ` + query EnumLiteralArgument { + widget(status: LEGACY_STATUS) { + name + } + } + `) + + expect(usages).toEqual([{ kind: 'enum-value', name: 'WidgetStatus.LEGACY_STATUS', deprecationReason: 'Use ACTIVE.', path: 'widget' }]) + }) + + it('records an enum value sent as a variable', () => { + const usages = collect( + /* GraphQL */ ` + query EnumViaVariable($status: WidgetStatus) { + widget(status: $status) { + name + } + } + `, + { variables: { status: 'LEGACY_STATUS' } }, + ) + + expect(usages).toEqual([ + { kind: 'enum-value', name: 'WidgetStatus.LEGACY_STATUS', deprecationReason: 'Use ACTIVE.', path: '$status' }, + ]) + }) + + it('records an enum value nested inside an input object sent as a variable', () => { + const usages = collect(saveWidgetViaVariable, { variables: { input: { status: 'LEGACY_STATUS' } } }) + + expect(usages).toEqual([ + { kind: 'enum-value', name: 'WidgetStatus.LEGACY_STATUS', deprecationReason: 'Use ACTIVE.', path: '$input.status' }, + ]) + }) + + it('records an enum value inside a list, with its index in the path', () => { + const usages = collect(saveWidgetViaVariable, { variables: { input: { statuses: ['ACTIVE', 'LEGACY_STATUS'] } } }) + + expect(usages).toEqual([ + { kind: 'enum-value', name: 'WidgetStatus.LEGACY_STATUS', deprecationReason: 'Use ACTIVE.', path: '$input.statuses.1' }, + ]) + }) + + it('records one entry however many list elements repeat the same value', () => { + const usages = collect(saveWidgetViaVariable, { + variables: { input: { statuses: Array.from({ length: 40 }, () => 'LEGACY_STATUS') } }, + }) + + expect(names(usages)).toEqual(['WidgetStatus.LEGACY_STATUS']) + }) + + it('records a deprecated enum value used as a variable definition default', () => { + const usages = collect(/* GraphQL */ ` + query EnumDefault($status: WidgetStatus = LEGACY_STATUS) { + widget(status: $status) { + name + } + } + `) + + expect(names(usages)).toEqual(['WidgetStatus.LEGACY_STATUS']) + }) + + it('records nothing for a non-deprecated enum value', () => { + expect(collect(saveWidgetViaVariable, { variables: { input: { status: 'ACTIVE' } } })).toEqual([]) + }) + + it('records nothing for an enum name that is not in the schema', () => { + // graphql-js rejects this at coercion, moments after the walk sees it; the walk must simply + // not invent a record for a value the enum doesn't define. + expect(collect(saveWidgetViaVariable, { variables: { input: { status: 'NOT_A_STATUS' } } })).toEqual([]) + }) + }) + + describe('result shape', () => { + it('carries the deprecation reason for every kind', () => { + const usages = collect( + /* GraphQL */ ` + mutation EveryKind($input: WidgetFilterInput!) { + saveWidget(input: $input) @audit(legacyTag: "x") { + legacyName + } + widget: saveWidget(input: { legacyId: "w1" }) { + name + } + } + `, + { variables: { input: { status: 'LEGACY_STATUS' } } }, + ) + + expect(usages.map(({ kind, deprecationReason }) => [kind, deprecationReason])).toEqual([ + ['directive-argument', 'Use tag.'], + ['enum-value', 'Use ACTIVE.'], + ['input-field', 'Use id.'], + ['output-field', 'Use name.'], + ]) + }) + + it('orders results by kind then name, whatever order they were found in', () => { + const usages = collect( + /* GraphQL */ ` + query Ordering($status: WidgetStatus) { + widget(legacyId: "w1", status: $status) { + legacyName + } + } + `, + { variables: { status: 'LEGACY_STATUS' } }, + ) + + expect(usages.map((usage) => usage.kind)).toEqual(['argument', 'enum-value', 'output-field']) + }) + + it('returns nothing for a schema with no deprecations at all', () => { + const plainSchema = buildSchema(/* GraphQL */ ` + type Query { + widget(id: ID): String + } + `) + + const usages = collectDeprecatedElementUsage({ + schema: plainSchema, + document: parse(/* GraphQL */ ` + { + widget(id: "w1") + } + `), + }) + + expect(usages).toEqual([]) + }) + }) + + describe('limits', () => { + it.each([ + ['records the deprecated field when nesting stays within the depth cap', 3, ['NestedInput.legacyFlag']], + ['stops at the depth cap instead of recursing without bound', 200, []], + ])('%s', (_label, depth, expected) => { + // Input types can be self-recursive (NestedInput.child here), so a hand-crafted payload can + // nest arbitrarily deep. The cap trades telemetry beyond that depth against blowing the stack. + const usages = collect(saveWidgetViaVariable, { variables: { input: { nested: nestChildren(depth) } } }) + + expect(names(usages)).toEqual(expected) + }) + + it('honours a non-default depth cap', () => { + const variables = { input: { nested: nestChildren(3) } } + + expect(names(collect(saveWidgetViaVariable, { variables, maxVariableDepth: 2 }))).toEqual([]) + expect(names(collect(saveWidgetViaVariable, { variables, maxVariableDepth: 10 }))).toEqual(['NestedInput.legacyFlag']) + }) + + it('stops walking variables once the node budget is spent', () => { + // The depth cap bounds how deep a payload goes, not how wide: a large flat list is shallow. + const variables = { input: { tags: [{ legacyLabel: 'gone' }] } } + + expect(names(collect(saveWidgetViaVariable, { variables, maxVariableNodes: 1 }))).toEqual([]) + expect(names(collect(saveWidgetViaVariable, { variables }))).toEqual(['TagInput.legacyLabel']) + }) + + it('caps the number of elements returned', () => { + const query = /* GraphQL */ ` + query Capped { + widget(legacyId: "w1") { + legacyName + } + } + ` + + expect(collect(query)).toHaveLength(2) + expect(collect(query, { maxElements: 1 })).toHaveLength(1) + }) + + it('terminates on a self-referential variable value', () => { + // A JSON request body can't express this, but an in-process caller can. + const cyclic: Record = {} + cyclic.child = cyclic + cyclic.legacyFlag = true + + expect(names(collect(saveWidgetViaVariable, { variables: { input: { nested: cyclic } } }))).toEqual(['NestedInput.legacyFlag']) + }) + }) + + describe('resilience', () => { + it.each([ + ['a scalar where an input object is declared', 'not-an-object'], + ['an array where an input object is declared', [{ legacyId: 'w1' }]], + ['a number where an input object is declared', 42], + ['a null where an input object is declared', null], + ['a boolean where an enum is declared', true], + ])('tolerates variables whose shape contradicts the declared type: %s', (_label, input) => { + // The walk runs before graphql-js coerces variables, so mismatched shapes genuinely reach it. + // It must record nothing rather than throw; a throw would turn the request into an error. + expect(() => collect(saveWidgetViaVariable, { variables: { input } })).not.toThrow() + expect(collect(saveWidgetViaVariable, { variables: { input } })).toEqual([]) + }) + + it('ignores variables that the operation does not declare', () => { + expect(collect(saveWidgetViaVariable, { variables: { input: { id: 'w1' }, stray: { legacyId: 'w1' } } })).toEqual([]) + }) + + it('records nothing when no variables are supplied for a declared variable', () => { + expect(collect(saveWidgetViaVariable)).toEqual([]) + }) + }) +}) diff --git a/src/deprecation.ts b/src/deprecation.ts new file mode 100644 index 0000000..5026bf2 --- /dev/null +++ b/src/deprecation.ts @@ -0,0 +1,317 @@ +import type { + ASTNode, + DocumentNode, + FieldNode, + GraphQLInputType, + GraphQLSchema, + ObjectFieldNode, + OperationDefinitionNode, + VariableDefinitionNode, +} from 'graphql' +import { + getNamedType, + getOperationAST, + isEnumType, + isInputObjectType, + isInputType, + isListType, + isNonNullType, + Kind, + separateOperations, + typeFromAST, + TypeInfo, + visit, + visitWithTypeInfo, +} from 'graphql' + +/** + * Safety valve for the variable walk. Recursion follows the schema's input types, which may be + * mutually recursive, and variables are walked before graphql-js coerces them — so nothing else + * bounds how deep a hand-crafted payload can go. Far above any real operation's nesting. + */ +export const DEFAULT_MAX_VARIABLE_DEPTH = 25 + +/** + * Bounds the *breadth* of the variable walk, which the depth cap does not: a single large array of + * small input objects is shallow but arbitrarily wide. + */ +export const DEFAULT_MAX_VARIABLE_NODES = 10_000 + +/** + * Caps the collected result so one hostile document cannot blow up a log entry. + */ +export const DEFAULT_MAX_ELEMENTS = 50 + +const UNKNOWN = '' + +export type DeprecatedElementKind = 'output-field' | 'argument' | 'directive-argument' | 'input-field' | 'enum-value' + +export interface DeprecatedElementUsage { + kind: DeprecatedElementKind + /** + * Stable key for aggregating usage, formatted per kind: + * `Type.field`, `Type.field(arg)`, `@directive(arg)`, `InputType.field`, `EnumType.VALUE`. + */ + name: string + /** + * The `reason` recorded on the element's `@deprecated` directive. + */ + deprecationReason: string + /** + * Best-effort location, for debugging a single record — not an aggregation key. A field selected + * inside a fragment definition has no enclosing field in its ancestry, so its path is relative to + * the fragment rather than the operation. + */ + path?: string +} + +export interface CollectDeprecatedElementUsageOptions { + schema: GraphQLSchema + document: DocumentNode + /** + * The executed operation. Resolved from `document` and `operationName` when omitted. + */ + operation?: OperationDefinitionNode | null + operationName?: string | null + /** + * Raw request variables, as supplied by the client and before graphql-js coerces them. + */ + variables?: Record | null + maxVariableDepth?: number + maxVariableNodes?: number + maxElements?: number +} + +/** + * Reports every `@deprecated` schema element an operation uses, across both the document and the + * supplied variables. + * + * Output fields and arguments are named in the document, so an AST walk finds them. Input fields + * and enum values are data the caller *supplies* rather than selects: they can arrive as a document + * literal, or inside a variable where the name appears nowhere in the AST, so both routes are + * walked and the results deduplicated. + * + * Results are deduplicated on kind and name — at most one entry per distinct element per request, + * bounded by the size of the schema rather than the size of the request — and sorted, so records + * are stable across requests. When the result reaches `maxElements` it is truncated, so callers + * should treat a result of exactly that length as possibly incomplete. + * + * The variable walk reads untrusted input before coercion, so a payload whose shape contradicts its + * declared type reaches it. Every shape it depends on is guarded: malformed variables yield no + * records rather than an error. + * + * This is request-side telemetry — it observes values a caller *sends*, never values the server + * returns. A deprecated element that only ever appears in responses reads as unused. + */ +export function collectDeprecatedElementUsage({ + schema, + document, + operation, + operationName, + variables, + maxVariableDepth = DEFAULT_MAX_VARIABLE_DEPTH, + maxVariableNodes = DEFAULT_MAX_VARIABLE_NODES, + maxElements = DEFAULT_MAX_ELEMENTS, +}: CollectDeprecatedElementUsageOptions): DeprecatedElementUsage[] { + const collected = new Map() + const collect = (usage: DeprecatedElementUsage): void => { + const key = `${usage.kind}:${usage.name}` + if (collected.size >= maxElements || collected.has(key)) return + collected.set(key, usage) + } + + const resolvedOperation = operation ?? getOperationAST(document, operationName ?? undefined) + + collectFromDocument(schema, scopeToOperation(document, resolvedOperation), collect) + collectFromVariables(schema, resolvedOperation?.variableDefinitions, variables, { maxVariableDepth, maxVariableNodes }, collect) + + return [...collected.values()].sort((a, b) => a.kind.localeCompare(b.kind) || a.name.localeCompare(b.name)) +} + +type Collect = (usage: DeprecatedElementUsage) => void + +/** + * A document may carry several operations but only one runs, so anything the others select would be + * reported as usage that never happened. `separateOperations` keeps each operation with only the + * fragments it can reach, which is exactly the scope wanted here. + */ +function scopeToOperation(document: DocumentNode, operation: OperationDefinitionNode | null | undefined): DocumentNode { + if (!operation) return document + + let operationCount = 0 + for (const definition of document.definitions) { + if (definition.kind === Kind.OPERATION_DEFINITION) operationCount++ + if (operationCount > 1) return separateOperations(document)[operation.name?.value ?? ''] ?? document + } + return document +} + +function collectFromDocument(schema: GraphQLSchema, document: DocumentNode, collect: Collect): void { + const typeInfo = new TypeInfo(schema) + + visit( + document, + visitWithTypeInfo(typeInfo, { + Field(_node, _key, _parent, _path, ancestors) { + const field = typeInfo.getFieldDef() + if (field?.deprecationReason == null) return + collect({ + kind: 'output-field', + name: `${typeInfo.getParentType()?.name ?? UNKNOWN}.${field.name}`, + deprecationReason: field.deprecationReason, + path: getPath(ancestors, field.name), + }) + }, + Argument(_node, _key, _parent, _path, ancestors) { + const argument = typeInfo.getArgument() + if (argument?.deprecationReason == null) return + // TypeInfo resolves an argument against the enclosing directive when there is one, so + // without this branch a directive's argument is reported as an argument of the field the + // directive is attached to — a field argument that does not exist. + const directive = typeInfo.getDirective() + collect({ + kind: directive ? 'directive-argument' : 'argument', + name: directive + ? `@${directive.name}(${argument.name})` + : `${typeInfo.getParentType()?.name ?? UNKNOWN}.${typeInfo.getFieldDef()?.name ?? UNKNOWN}(${argument.name})`, + deprecationReason: argument.deprecationReason, + path: getPath(ancestors), + }) + }, + // Input-object fields written as literals in the document, including variable definition + // default values, which TypeInfo also tracks as input positions. + ObjectField(node, _key, _parent, _path, ancestors) { + const parentType = getNamedType(typeInfo.getParentInputType()) + if (!isInputObjectType(parentType)) return + const inputField = parentType.getFields()[node.name.value] + if (inputField?.deprecationReason == null) return + collect({ + kind: 'input-field', + name: `${parentType.name}.${inputField.name}`, + deprecationReason: inputField.deprecationReason, + path: getPath(ancestors, inputField.name), + }) + }, + EnumValue(node, _key, _parent, _path, ancestors) { + const enumType = getNamedType(typeInfo.getInputType()) + if (!isEnumType(enumType)) return + const enumValue = enumType.getValue(node.value) + if (enumValue?.deprecationReason == null) return + collect({ + kind: 'enum-value', + name: `${enumType.name}.${enumValue.name}`, + deprecationReason: enumValue.deprecationReason, + path: getPath(ancestors), + }) + }, + }), + ) +} + +type PathNode = FieldNode | ObjectFieldNode + +function isPathNode(node: ASTNode): node is PathNode { + return node.kind === Kind.FIELD || node.kind === Kind.OBJECT_FIELD +} + +function getPath(ancestors: readonly (ASTNode | readonly ASTNode[])[], leaf?: string): string { + const segments = ancestors + .filter((x): x is ASTNode => !Array.isArray(x)) + .filter(isPathNode) + .map((x) => x.name.value) + + if (leaf) segments.push(leaf) + + return segments.join('.') +} + +interface WalkLimits { + maxVariableDepth: number + maxVariableNodes: number +} + +/** + * Walks each supplied variable value against its declared type, reporting every deprecated input + * field and enum value the client actually sent. + * + * Takes the definitions of the *resolved* operation rather than scanning the whole document: the + * same variable name may be declared with a different type in each operation, so only the executed + * one's declarations describe the supplied values. + */ +function collectFromVariables( + schema: GraphQLSchema, + variableDefinitions: readonly VariableDefinitionNode[] | undefined, + variables: Record | null | undefined, + limits: WalkLimits, + collect: Collect, +): void { + if (variables == null) return + + const budget = { remaining: limits.maxVariableNodes } + + for (const variableDefinition of variableDefinitions ?? []) { + const variableName = variableDefinition.variable.name.value + if (!Object.hasOwn(variables, variableName)) continue + + const type = typeFromAST(schema, variableDefinition.type) + if (!isInputType(type)) continue + + walkInputValue(type, variables[variableName], `$${variableName}`, 0, limits, budget, collect) + } +} + +function walkInputValue( + type: GraphQLInputType, + value: unknown, + path: string, + depth: number, + limits: WalkLimits, + budget: { remaining: number }, + collect: Collect, +): void { + if (depth > limits.maxVariableDepth || budget.remaining <= 0 || value == null) return + budget.remaining-- + + // Unwrapped inline rather than by recursing, so a nullability wrapper doesn't spend a node from + // the budget for a value that has not been descended into yet. + const unwrapped = isNonNullType(type) ? type.ofType : type + + if (isListType(unwrapped)) { + // GraphQL accepts a bare value in a list position, so handle both shapes. + const items = Array.isArray(value) ? value : [value] + for (const [index, item] of items.entries()) { + walkInputValue(unwrapped.ofType, item, Array.isArray(value) ? `${path}.${index}` : path, depth + 1, limits, budget, collect) + } + return + } + + if (isEnumType(unwrapped)) { + // An unknown name is a coercion error graphql-js raises moments later; nothing to record here. + if (typeof value !== 'string') return + const enumValue = unwrapped.getValue(value) + if (enumValue?.deprecationReason != null) { + collect({ kind: 'enum-value', name: `${unwrapped.name}.${enumValue.name}`, deprecationReason: enumValue.deprecationReason, path }) + } + return + } + + // Scalars have no members that could carry a deprecation. + if (!isInputObjectType(unwrapped) || !isRecord(value)) return + + for (const field of Object.values(unwrapped.getFields())) { + if (!Object.hasOwn(value, field.name)) continue + + const fieldPath = `${path}.${field.name}` + // Presence is the signal, explicit `null` included: a client still sending the field would + // break if it were removed from the schema. + if (field.deprecationReason != null) { + collect({ kind: 'input-field', name: `${unwrapped.name}.${field.name}`, deprecationReason: field.deprecationReason, path: fieldPath }) + } + + walkInputValue(field.type, value[field.name], fieldPath, depth + 1, limits, budget, collect) + } +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} diff --git a/src/index.ts b/src/index.ts index 988cd95..da5119e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,5 @@ export * from './context' +export * from './deprecation' export * from './logging' export * from './operation' export * from './request-info' diff --git a/src/logging.spec.ts b/src/logging.spec.ts new file mode 100644 index 0000000..0103cfa --- /dev/null +++ b/src/logging.spec.ts @@ -0,0 +1,46 @@ +import type { Logger } from '@makerx/node-common' +import { describe, expect, it, vi } from 'vitest' +import type { DeprecatedElementUsage } from './deprecation' +import { logGraphQLOperation } from './logging' + +const makeLogger = () => ({ error: vi.fn(), warn: vi.fn(), info: vi.fn(), verbose: vi.fn(), debug: vi.fn() }) satisfies Logger + +const loggedEntry = (logger: ReturnType): Record => + logger.info.mock.calls[0]?.[1] as Record + +const usage: DeprecatedElementUsage = { + kind: 'output-field', + name: 'Widget.legacyName', + deprecationReason: 'Use name.', + path: 'widget.legacyName', +} + +describe('logGraphQLOperation', () => { + describe('deprecatedElements', () => { + it('includes the elements when any were collected', () => { + const logger = makeLogger() + + logGraphQLOperation({ logger, operationName: 'GetWidget', deprecatedElements: [usage] }) + + expect(loggedEntry(logger).deprecatedElements).toEqual([usage]) + }) + + it('omits the key entirely when the collection came back empty', () => { + // `omitBy(..., isNil)` keeps an empty array, so every operation would otherwise carry a + // `deprecatedElements: []` that means nothing. + const logger = makeLogger() + + logGraphQLOperation({ logger, operationName: 'GetWidget', deprecatedElements: [] }) + + expect(loggedEntry(logger)).not.toHaveProperty('deprecatedElements') + }) + + it('omits the key when collection was not requested', () => { + const logger = makeLogger() + + logGraphQLOperation({ logger, operationName: 'GetWidget' }) + + expect(loggedEntry(logger)).not.toHaveProperty('deprecatedElements') + }) + }) +}) diff --git a/src/logging.ts b/src/logging.ts index 9b149cc..9dfc136 100644 --- a/src/logging.ts +++ b/src/logging.ts @@ -5,6 +5,8 @@ import type { ExecutionArgs, GraphQLFormattedError } from 'graphql' import { OperationTypeNode, print } from 'graphql' import type { ExecutionResult } from 'graphql-ws' import type { GraphQLContext } from './context' +import type { DeprecatedElementUsage } from './deprecation' +import { collectDeprecatedElementUsage } from './deprecation' import { isIntrospectionQuery } from './operation' import { isNil } from './utils' @@ -63,6 +65,11 @@ export interface GraphQLLogOperationInfo extend * Whether the operation is a subsequent payload of an incremental response. */ isSubsequentPayload?: boolean + /** + * Deprecated schema elements the operation used, as returned by `collectDeprecatedElementUsage`. + * Omitted from the log entry when empty. + */ + deprecatedElements?: DeprecatedElementUsage[] /** * The logger to use. */ @@ -85,6 +92,7 @@ export const logGraphQLOperation = ({ query, variables, result, + deprecatedElements, logger, logLevel = 'info', ...rest @@ -102,6 +110,7 @@ export const logGraphQLOperation = ({ duration: started ? Date.now() - started : undefined, result: result ? omitBy(result, isNil) : undefined, isIntrospectionQuery: isIntrospection || undefined, + deprecatedElements: deprecatedElements?.length ? deprecatedElements : undefined, ...omitBy(rest, isNil), }, isNil, @@ -116,6 +125,7 @@ export const logSubscriptionOperation = ({ message, logLevel, resolveLogger, + includeDeprecatedElements, }: { id?: string args: ExecutionArgs @@ -123,15 +133,28 @@ export const logSubscriptionOperation = ({ message?: string logLevel?: keyof LoggerLogFunctions resolveLogger?: (context: GraphQLContext) => TLogger + /** + * If true, deprecated schema elements the operation uses are collected and logged. + */ + includeDeprecatedElements?: boolean }) => { const logger = resolveLogger ? resolveLogger(args.contextValue as GraphQLContext) : ((args.contextValue as GraphQLContext).logger as TLogger) if (!logger) return - const { operationName, variableValues, document } = args + const { operationName, variableValues, document, schema } = args const { data, ...resultWithoutData } = result ?? {} + let deprecatedElements: DeprecatedElementUsage[] | undefined + if (includeDeprecatedElements) { + try { + deprecatedElements = collectDeprecatedElementUsage({ schema, document, operationName, variables: variableValues }) + } catch (error) { + logger.warn('Failed to collect deprecated schema element usage', { error, operationName }) + } + } + logGraphQLOperation({ message, type: OperationTypeNode.SUBSCRIPTION, @@ -140,6 +163,7 @@ export const logSubscriptionOperation = ({ query: print(document), variables: variableValues, result: resultWithoutData, + deprecatedElements, logger, logLevel, }) diff --git a/src/subscriptions/server.ts b/src/subscriptions/server.ts index 355ce6f..8b1494f 100644 --- a/src/subscriptions/server.ts +++ b/src/subscriptions/server.ts @@ -21,6 +21,7 @@ export function useSubscriptionsServer({ requireAuth, jwtClaimsToLog = ['oid', 'iss'], resolveSubscriptionOperationLogger, + includeDeprecatedElements, }: { schema: GraphQLSchema httpServer: Server @@ -32,6 +33,11 @@ export function useSubscriptionsServer({ requireAuth?: boolean jwtClaimsToLog?: string[] resolveSubscriptionOperationLogger?: (context: GraphQLContext) => TLogger + /** + * If true, deprecated schema elements a subscription uses are collected and logged when it is + * established. Not collected per emitted payload: the usage is a property of the operation. + */ + includeDeprecatedElements?: boolean }) { if (requireAuth && !verifyToken) throw new Error('verifyToken must be supplied when requireAuth is true') @@ -103,7 +109,13 @@ export function useSubscriptionsServer({ }) }, onOperation(_ctx, id, _payload, args) { - logSubscriptionOperation({ id, args, logLevel: operationLogLevel, resolveLogger: resolveSubscriptionOperationLogger }) + logSubscriptionOperation({ + id, + args, + logLevel: operationLogLevel, + resolveLogger: resolveSubscriptionOperationLogger, + includeDeprecatedElements, + }) }, onNext(_ctx, id, _payload, args, { data, ...result }) { logSubscriptionOperation({ id, args, logLevel: operationLogLevel, result, resolveLogger: resolveSubscriptionOperationLogger }) From 65692a7b05ef34c145753bfba28d327a660cb67c Mon Sep 17 00:00:00 2001 From: Sam Curry Date: Mon, 17 Aug 2026 12:10:51 +0800 Subject: [PATCH 2/5] chore(deps): resolve dev-dependency audit advisories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `npm run audit` gates CI and had started failing on seven advisories published since the last run on main — none introduced here, and none reaching consumers: this package has no runtime dependencies, so every affected path is a devDependency or a peer. `fast-uri` arrives via better-npm-audit's own dependency tree. `npm audit fix` resolved all seven within the existing semver ranges, so only the lockfile changes. Co-Authored-By: Claude Opus 5 (1M context) --- package-lock.json | 179 ++++++++++++++++++++++++---------------------- 1 file changed, 94 insertions(+), 85 deletions(-) diff --git a/package-lock.json b/package-lock.json index ab49eb4..89b2142 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@makerx/graphql-core", - "version": "3.2.0", + "version": "3.3.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@makerx/graphql-core", - "version": "3.2.0", + "version": "3.3.0", "license": "MIT", "devDependencies": { "@arethetypeswrong/cli": "^0.18.2", @@ -2501,16 +2501,16 @@ } }, "node_modules/@typescript-eslint/typescript-estree/node_modules/brace-expansion": { - "version": "5.0.6", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.6.tgz", - "integrity": "sha512-kLpxurY4Z4r9sgMsyG0Z9uzsBlgiU/EFKhj/h91/8yHu0edo7XuixOIH3VcJ8kkxs6/jPzoI6U9Vj3WqbMQ94g==", + "version": "5.0.9", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.9.tgz", + "integrity": "sha512-ScQ4IuvIEF1TMlP7Zt+vjJ//9zlPb2SDcxWxM3bk8s6t6GGdJ7KO1dCcTidOPJKePW30LE/2cT7wCyPho9/Wxg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/@typescript-eslint/typescript-estree/node_modules/minimatch": { @@ -2983,21 +2983,21 @@ } }, "node_modules/body-parser": { - "version": "2.2.2", - "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-2.2.2.tgz", - "integrity": "sha512-oP5VkATKlNwcgvxi0vM0p/D3n2C3EReYVX+DNYs5TjZFn/oQt2j+4sVJtSMr18pdRr8wjTcBl6LoV+FUwzPmNA==", + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-2.3.0.tgz", + "integrity": "sha512-2cGmJupaNgg+QUwVLAucDuWuoMZ6EX9iHDRswZ5lsNYEmwPaRknMPCLZz07yTzVq/83p4o/wzbDZbBrTvGGTIw==", "dev": true, "license": "MIT", "dependencies": { "bytes": "^3.1.2", - "content-type": "^1.0.5", + "content-type": "^2.0.0", "debug": "^4.4.3", - "http-errors": "^2.0.0", - "iconv-lite": "^0.7.0", + "http-errors": "^2.0.1", + "iconv-lite": "^0.7.2", "on-finished": "^2.4.1", - "qs": "^6.14.1", - "raw-body": "^3.0.1", - "type-is": "^2.0.1" + "qs": "^6.15.2", + "raw-body": "^3.0.2", + "type-is": "^2.1.0" }, "engines": { "node": ">=18" @@ -3007,10 +3007,24 @@ "url": "https://opencollective.com/express" } }, + "node_modules/body-parser/node_modules/content-type": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/content-type/-/content-type-2.1.0.tgz", + "integrity": "sha512-mj7UPXE0jaqaOsukNZRUEfEi2AcL7C/vwmwcHV0O97eO1E1pxBZuyjlZrx5seTaNBg1U6+o35wpa35Qfcc+7ag==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" + } + }, "node_modules/brace-expansion": { - "version": "1.1.14", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.14.tgz", - "integrity": "sha512-MWPGfDxnyzKU7rNOW9SP/c50vi3xrmrua/+6hfPbCS2ABNWfx24vPidzvC7krjU/RTo235sV776ymlsMtGKj8g==", + "version": "1.1.18", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz", + "integrity": "sha512-Edep/X9fGqVNmzKBVsDYIOtD+z1tuezV70LBjdCst9Tqu76lsnvRiZ6oTic1n+/BIwX6QDGAO94PN4N2SADvtw==", "dev": true, "license": "MIT", "dependencies": { @@ -4124,9 +4138,9 @@ "license": "MIT" }, "node_modules/fast-uri": { - "version": "3.1.2", - "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.2.tgz", - "integrity": "sha512-rVjf7ArG3LTk+FS6Yw81V1DLuZl1bRbNrev6Tmd/9RaroeeRRJhAt7jg/6YFxbvAQXUCavSoZhPPj6oOx+5KjQ==", + "version": "3.1.5", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.5.tgz", + "integrity": "sha512-gHwA1O9LDIcKunMKhObS/HimwtehO1nPUECKAu5TpKgaO19fcWEl4bliWe1jWxVFvIXztJjjQ4L8XQ1EU9f7Jw==", "dev": true, "funding": [ { @@ -4438,16 +4452,16 @@ } }, "node_modules/glob/node_modules/brace-expansion": { - "version": "5.0.6", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.6.tgz", - "integrity": "sha512-kLpxurY4Z4r9sgMsyG0Z9uzsBlgiU/EFKhj/h91/8yHu0edo7XuixOIH3VcJ8kkxs6/jPzoI6U9Vj3WqbMQ94g==", + "version": "5.0.9", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.9.tgz", + "integrity": "sha512-ScQ4IuvIEF1TMlP7Zt+vjJ//9zlPb2SDcxWxM3bk8s6t6GGdJ7KO1dCcTidOPJKePW30LE/2cT7wCyPho9/Wxg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/glob/node_modules/minimatch": { @@ -4687,30 +4701,24 @@ "license": "MIT" }, "node_modules/http-errors": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-2.0.0.tgz", - "integrity": "sha512-FtwrG/euBzaEjYeRqOgly7G0qviiXoJWnvEH2Z1plBdXgbyjv34pHTSb9zoeHMyDy33+DWy5Wt9Wo+TURtOYSQ==", + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-2.0.1.tgz", + "integrity": "sha512-4FbRdAX+bSdmo4AUFuS0WNiPz8NgFt+r8ThgNWmlrjQjt1Q7ZR9+zTlce2859x4KSXrwIsaeTqDoKQmtP8pLmQ==", "dev": true, "license": "MIT", "dependencies": { - "depd": "2.0.0", - "inherits": "2.0.4", - "setprototypeof": "1.2.0", - "statuses": "2.0.1", - "toidentifier": "1.0.1" + "depd": "~2.0.0", + "inherits": "~2.0.4", + "setprototypeof": "~1.2.0", + "statuses": "~2.0.2", + "toidentifier": "~1.0.1" }, "engines": { "node": ">= 0.8" - } - }, - "node_modules/http-errors/node_modules/statuses": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.1.tgz", - "integrity": "sha512-RwNA9Z/7PrK06rYLIzFMlaF+l73iwpzsqRIFgbMLbTcLD6cOao82TaWefPXQvB2fOC4AjuYSEndS7N/mTCbkdQ==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">= 0.8" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" } }, "node_modules/iconv-lite": { @@ -5118,9 +5126,9 @@ "dev": true }, "node_modules/js-yaml": { - "version": "4.2.0", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.2.0.tgz", - "integrity": "sha512-ePWsvanv0DWuDRsW8dnt+R4jQ31SCRCQ7hhNcPXZPsoBZiemuZNYGf7adZdqX2D86j6rvKp3RpCxVTSb8WQlOw==", + "version": "4.3.1", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.1.tgz", + "integrity": "sha512-CY6crGq313MX8GkwvB7tzgp99vjQxY1++5y10/BKN/GUfHqWaOGQMNZkBvqSzsZKWk/ijwHlWzzkLulsGHhjWQ==", "dev": true, "funding": [ { @@ -5788,9 +5796,9 @@ "dev": true }, "node_modules/nanoid": { - "version": "3.3.15", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.15.tgz", - "integrity": "sha512-y7Wygv/7mEOvxTuEQDB8StXdMRBWf1kR/tlhAzBRUFkB2jfcLOAxO/SHmOO2zgz1pVgK29/kyupn059/bCHdjA==", + "version": "3.3.18", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.18.tgz", + "integrity": "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==", "dev": true, "funding": [ { @@ -6373,9 +6381,9 @@ } }, "node_modules/postcss": { - "version": "8.5.15", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.15.tgz", - "integrity": "sha512-FfR8sjd4em2T6fb3I2MwAJU7HWVMr9zba+enmQeeWFfCbm+UOC/0X4DS8XtpUTMwWMGbjKYP7xjfNekzyGmB3A==", + "version": "8.5.26", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.26.tgz", + "integrity": "sha512-u82N74LFzG8ca+dD8puPnplTXoGH4fTPpVGuIbt36G3qvNlkvfD0lEAZSxaly3KX8TS/L1A1gsCEmvKmBcVbkQ==", "dev": true, "funding": [ { @@ -6393,7 +6401,7 @@ ], "license": "MIT", "dependencies": { - "nanoid": "^3.3.12", + "nanoid": "^3.3.17", "picocolors": "^1.1.1", "source-map-js": "^1.2.1" }, @@ -6501,38 +6509,21 @@ } }, "node_modules/raw-body": { - "version": "3.0.1", - "resolved": "https://registry.npmjs.org/raw-body/-/raw-body-3.0.1.tgz", - "integrity": "sha512-9G8cA+tuMS75+6G/TzW8OtLzmBDMo8p1JRxN5AZ+LAp8uxGA8V8GZm4GQ4/N5QNQEnLmg6SS7wyuSmbKepiKqA==", + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/raw-body/-/raw-body-3.0.2.tgz", + "integrity": "sha512-K5zQjDllxWkf7Z5xJdV0/B0WTNqx6vxG70zJE4N0kBs4LovmEYWJzQGxC9bS9RAKu3bgM40lrd5zoLJ12MQ5BA==", "dev": true, "license": "MIT", "dependencies": { - "bytes": "3.1.2", - "http-errors": "2.0.0", - "iconv-lite": "0.7.0", - "unpipe": "1.0.0" + "bytes": "~3.1.2", + "http-errors": "~2.0.1", + "iconv-lite": "~0.7.0", + "unpipe": "~1.0.0" }, "engines": { "node": ">= 0.10" } }, - "node_modules/raw-body/node_modules/iconv-lite": { - "version": "0.7.0", - "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.7.0.tgz", - "integrity": "sha512-cf6L2Ds3h57VVmkZe+Pn+5APsT7FpqJtEhhieDCvrE2MK5Qk9MyffgQyuxQTm6BChfeZNtcOLHp9IcWRVcIcBQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "safer-buffer": ">= 2.1.2 < 3.0.0" - }, - "engines": { - "node": ">=0.10.0" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/express" - } - }, "node_modules/read-pkg": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/read-pkg/-/read-pkg-3.0.0.tgz", @@ -6954,9 +6945,9 @@ } }, "node_modules/shell-quote": { - "version": "1.8.4", - "resolved": "https://registry.npmjs.org/shell-quote/-/shell-quote-1.8.4.tgz", - "integrity": "sha512-VsC6n6vz1ihYYyZZwX7YZSF5l5x36ca17OC+a69h94YqB7X6XLwf+5MOgynYir2SLFUbl8gIYvBo8K8RoNQ6bQ==", + "version": "1.10.0", + "resolved": "https://registry.npmjs.org/shell-quote/-/shell-quote-1.10.0.tgz", + "integrity": "sha512-w1aiOKwKuRgtwAReIIj89puqg+I7GvX4IbLrvmhXbzQsj1+Zwi4VO3+fa6ZF91TWSjIxoEkKnMeHcLEODK5ZXA==", "dev": true, "license": "MIT", "engines": { @@ -7539,18 +7530,36 @@ } }, "node_modules/type-is": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/type-is/-/type-is-2.0.1.tgz", - "integrity": "sha512-OZs6gsjF4vMp32qrCbiVSkrFmXtG/AZhY3t0iAMrMBiAZyV9oALtXO8hsrHbMXF9x6L3grlFuwW2oAz7cav+Gw==", + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/type-is/-/type-is-2.1.0.tgz", + "integrity": "sha512-faYHw0anBbc/kWF3zFTEnxSFOAGUX9GFbOBthvDdLsIlEoWOFOtS0zgCiQYwIskL9iGXZL3kAXD8OoZ4GmMATA==", "dev": true, "license": "MIT", "dependencies": { - "content-type": "^1.0.5", + "content-type": "^2.0.0", "media-typer": "^1.1.0", "mime-types": "^3.0.0" }, "engines": { - "node": ">= 0.6" + "node": ">= 18" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" + } + }, + "node_modules/type-is/node_modules/content-type": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/content-type/-/content-type-2.1.0.tgz", + "integrity": "sha512-mj7UPXE0jaqaOsukNZRUEfEi2AcL7C/vwmwcHV0O97eO1E1pxBZuyjlZrx5seTaNBg1U6+o35wpa35Qfcc+7ag==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" } }, "node_modules/typed-array-buffer": { From b3f203daf2195340b8c0a088b25e1095f50728db Mon Sep 17 00:00:00 2001 From: Sam Curry Date: Mon, 17 Aug 2026 12:39:01 +0800 Subject: [PATCH 3/5] fix(deprecation): stop walking a list once the node budget is spent The recursive call returned early once the budget was exhausted, but the loop around it did not: list length comes from the payload, and each step built a path string before making the no-op call. A large flat array therefore still cost work proportional to its length, which is what `maxVariableNodes` exists to prevent. Measured on a 5,000 element list with a budget of 10: 5,000 element reads before, fewer than 50 after. The input-object loop gets the same guard. It is bounded by schema size rather than payload size so the cost was never unbounded there, but it makes the rule uniform: once the budget is spent, traversal stops. Co-Authored-By: Claude Opus 5 (1M context) --- src/deprecation.spec.ts | 19 +++++++++++++++++++ src/deprecation.ts | 4 ++++ 2 files changed, 23 insertions(+) diff --git a/src/deprecation.spec.ts b/src/deprecation.spec.ts index ffb6207..3a190bd 100644 --- a/src/deprecation.spec.ts +++ b/src/deprecation.spec.ts @@ -604,6 +604,25 @@ describe('collectDeprecatedElementUsage', () => { expect(names(collect(saveWidgetViaVariable, { variables }))).toEqual(['TagInput.legacyLabel']) }) + it('stops iterating a list once the node budget is spent, rather than running it out', () => { + // The recursive call returns immediately once the budget is gone, but the loop around it is + // the unbounded part: list length comes from the payload, and each step builds a path string. + let indexReads = 0 + const tags = new Proxy( + Array.from({ length: 5_000 }, (_, i) => ({ legacyLabel: `tag-${i}` })), + { + get(target, property, receiver) { + if (typeof property === 'string' && /^\d+$/.test(property)) indexReads++ + return Reflect.get(target, property, receiver) + }, + }, + ) + + collect(saveWidgetViaVariable, { variables: { input: { tags } }, maxVariableNodes: 10 }) + + expect(indexReads).toBeLessThan(50) + }) + it('caps the number of elements returned', () => { const query = /* GraphQL */ ` query Capped { diff --git a/src/deprecation.ts b/src/deprecation.ts index 5026bf2..2af9155 100644 --- a/src/deprecation.ts +++ b/src/deprecation.ts @@ -280,6 +280,9 @@ function walkInputValue( // GraphQL accepts a bare value in a list position, so handle both shapes. const items = Array.isArray(value) ? value : [value] for (const [index, item] of items.entries()) { + // Checked here rather than relying on the recursive call returning early: the loop itself is + // the unbounded part, since list length comes from the payload and each step builds a path. + if (budget.remaining <= 0) return walkInputValue(unwrapped.ofType, item, Array.isArray(value) ? `${path}.${index}` : path, depth + 1, limits, budget, collect) } return @@ -299,6 +302,7 @@ function walkInputValue( if (!isInputObjectType(unwrapped) || !isRecord(value)) return for (const field of Object.values(unwrapped.getFields())) { + if (budget.remaining <= 0) return if (!Object.hasOwn(value, field.name)) continue const fieldPath = `${path}.${field.name}` From 771a41661ce0fc922773697ff67ee3a096edf869 Mon Sep 17 00:00:00 2001 From: Sam Curry Date: Mon, 17 Aug 2026 13:50:55 +0800 Subject: [PATCH 4/5] fix(deprecation)!: report truncation from every limit, not just the element cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Collection stops at three limits, but only `maxElements` was visible to callers — and only by inferring it from the result length, which also misreads an exactly-at-the-cap complete result as truncated. Hitting `maxVariableDepth` or `maxVariableNodes` produced a short list that looked complete. That is the dangerous direction for this telemetry. Its purpose is answering "does anything still use this element", so a silently truncated result invites the answer "no", which is how a still-used element gets deleted. `maxVariableNodes` is reachable by a legitimate request: a large batch mutation is shallow but wide. `collectDeprecatedElementUsage` now returns `{ elements, truncated }`, with `truncated` set by any of the three limits. Deduplication and null values do not set it — neither loses anything. Consumers see it as `deprecatedElementsTruncated` on the log entry, omitted when false. BREAKING CHANGE: collectDeprecatedElementUsage returns { elements, truncated } rather than an array. Unreleased, so no published version is affected. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 8 ++-- src/deprecation.spec.ts | 56 +++++++++++++++++++++++++++- src/deprecation.ts | 81 +++++++++++++++++++++++++++-------------- src/logging.ts | 14 ++++++- 4 files changed, 126 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index aa8f95c..8304fe6 100644 --- a/README.md +++ b/README.md @@ -288,8 +288,9 @@ This function can be used across implementations, e.g. in a [GraphQL Envelop plu Reports every `@deprecated` schema element an operation uses, so you can tell whether a deprecated element is safe to remove. ```ts -const deprecatedElements = collectDeprecatedElementUsage({ schema, document, operation, operationName, variables }) -// [{ kind: 'input-field', name: 'WidgetFilterInput.legacyId', deprecationReason: 'Use id.', path: '$input.legacyId' }] +const { elements, truncated } = collectDeprecatedElementUsage({ schema, document, operation, operationName, variables }) +// elements: [{ kind: 'input-field', name: 'WidgetFilterInput.legacyId', deprecationReason: 'Use id.', path: '$input.legacyId' }] +// truncated: false ``` Five kinds are detected: `output-field`, `argument`, `directive-argument`, `input-field` and `enum-value`. @@ -304,7 +305,8 @@ Behaviour worth knowing: - **Only the executed operation counts.** A document may hold several operations but only one runs, so the others — and any fragments only they reach — are excluded. - **Presence is the signal, explicit `null` included.** A client still sending a field would break if it were removed. - **It never throws on malformed variables.** The walk runs before graphql-js coerces variables, so a payload whose shape contradicts its declared type reaches it; such values yield no records rather than an error. -- **Work is bounded** by `maxVariableDepth` (default 25), `maxVariableNodes` (default 10,000) and `maxElements` (default 50). A result of exactly `maxElements` should be treated as possibly incomplete. +- **Work is bounded** by `maxVariableDepth` (default 25), `maxVariableNodes` (default 10,000) and `maxElements` (default 50). Whenever any of them stops collection early, `truncated` is `true`. +- **`truncated` guards against a false negative.** Absence from `elements` only means an element went unused when `truncated` is `false`; on a truncated result the walk stopped before it finished. This matters because reading "nothing uses this" off an incomplete result is exactly how a still-used element gets deleted. Deduplication and null values do not set it — nothing is lost in either case. Two limitations to design around: diff --git a/src/deprecation.spec.ts b/src/deprecation.spec.ts index 3a190bd..98a1bd4 100644 --- a/src/deprecation.spec.ts +++ b/src/deprecation.spec.ts @@ -81,6 +81,10 @@ const schema = buildSchema(typeDefs) type CollectOverrides = Omit function collect(query: string, overrides: CollectOverrides = {}): DeprecatedElementUsage[] { + return collectDeprecatedElementUsage({ schema, document: parse(query), ...overrides }).elements +} + +function collectResult(query: string, overrides: CollectOverrides = {}) { return collectDeprecatedElementUsage({ schema, document: parse(query), ...overrides }) } @@ -564,7 +568,7 @@ describe('collectDeprecatedElementUsage', () => { } `) - const usages = collectDeprecatedElementUsage({ + const usage = collectDeprecatedElementUsage({ schema: plainSchema, document: parse(/* GraphQL */ ` { @@ -573,7 +577,7 @@ describe('collectDeprecatedElementUsage', () => { `), }) - expect(usages).toEqual([]) + expect(usage).toEqual({ elements: [], truncated: false }) }) }) @@ -623,6 +627,54 @@ describe('collectDeprecatedElementUsage', () => { expect(indexReads).toBeLessThan(50) }) + /** + * Truncation is reported because the question this telemetry answers is "does anything still + * use this element". A truncated result that looks complete invites the answer "no", which is + * how a still-used element gets deleted — so every limit that can hide usage has to say so. + */ + describe('reports truncation', () => { + it('is false when nothing was cut short', () => { + expect(collectResult(saveWidgetViaVariable, { variables: { input: { legacyId: 'w1' } } })).toMatchObject({ truncated: false }) + }) + + it('is true when the element cap is reached', () => { + const query = /* GraphQL */ ` + query Capped { + widget(legacyId: "w1") { + legacyName + } + } + ` + + expect(collectResult(query, { maxElements: 1 }).truncated).toBe(true) + expect(collectResult(query, { maxElements: 2 }).truncated).toBe(false) + }) + + it('is true when the depth cap stops the walk', () => { + expect(collectResult(saveWidgetViaVariable, { variables: { input: { nested: nestChildren(200) } } })).toMatchObject({ + elements: [], + truncated: true, + }) + }) + + it('is true when the node budget stops the walk', () => { + const variables = { input: { tags: [{ legacyLabel: 'gone' }] } } + + expect(collectResult(saveWidgetViaVariable, { variables, maxVariableNodes: 1 })).toMatchObject({ elements: [], truncated: true }) + }) + + it('is not raised by deduplication, which loses nothing', () => { + // Every repeat past the first is dropped as a duplicate, not as truncation. + const variables = { input: { tags: Array.from({ length: 40 }, () => ({ legacyLabel: 'x' })) } } + + expect(collectResult(saveWidgetViaVariable, { variables })).toMatchObject({ truncated: false }) + }) + + it('is not raised by a null value, which has nothing to walk', () => { + expect(collectResult(saveWidgetViaVariable, { variables: { input: { nested: null } } })).toMatchObject({ truncated: false }) + }) + }) + it('caps the number of elements returned', () => { const query = /* GraphQL */ ` query Capped { diff --git a/src/deprecation.ts b/src/deprecation.ts index 2af9155..d6e2dd3 100644 --- a/src/deprecation.ts +++ b/src/deprecation.ts @@ -82,6 +82,16 @@ export interface CollectDeprecatedElementUsageOptions { maxElements?: number } +export interface CollectDeprecatedElementUsageResult { + elements: DeprecatedElementUsage[] + /** + * True when a limit — `maxElements`, `maxVariableDepth` or `maxVariableNodes` — stopped + * collection before it finished, so `elements` may be missing elements the operation really used. + * Absence from a truncated result is not evidence that an element is unused. + */ + truncated: boolean +} + /** * Reports every `@deprecated` schema element an operation uses, across both the document and the * supplied variables. @@ -93,8 +103,11 @@ export interface CollectDeprecatedElementUsageOptions { * * Results are deduplicated on kind and name — at most one entry per distinct element per request, * bounded by the size of the schema rather than the size of the request — and sorted, so records - * are stable across requests. When the result reaches `maxElements` it is truncated, so callers - * should treat a result of exactly that length as possibly incomplete. + * are stable across requests. + * + * Returns `truncated` when any limit stopped collection. Absence from `elements` is only evidence + * that an element went unused when `truncated` is false — which matters, because concluding + * "nothing uses this" from a truncated result is how a still-used element gets deleted. * * The variable walk reads untrusted input before coercion, so a payload whose shape contradicts its * declared type reaches it. Every shape it depends on is guarded: malformed variables yield no @@ -112,20 +125,30 @@ export function collectDeprecatedElementUsage({ maxVariableDepth = DEFAULT_MAX_VARIABLE_DEPTH, maxVariableNodes = DEFAULT_MAX_VARIABLE_NODES, maxElements = DEFAULT_MAX_ELEMENTS, -}: CollectDeprecatedElementUsageOptions): DeprecatedElementUsage[] { +}: CollectDeprecatedElementUsageOptions): CollectDeprecatedElementUsageResult { const collected = new Map() + const walk: WalkState = { maxVariableDepth, remaining: maxVariableNodes, truncated: false } + const collect = (usage: DeprecatedElementUsage): void => { const key = `${usage.kind}:${usage.name}` - if (collected.size >= maxElements || collected.has(key)) return + // Already recorded is deduplication, not truncation — only a rejected *new* element is a loss. + if (collected.has(key)) return + if (collected.size >= maxElements) { + walk.truncated = true + return + } collected.set(key, usage) } const resolvedOperation = operation ?? getOperationAST(document, operationName ?? undefined) collectFromDocument(schema, scopeToOperation(document, resolvedOperation), collect) - collectFromVariables(schema, resolvedOperation?.variableDefinitions, variables, { maxVariableDepth, maxVariableNodes }, collect) + collectFromVariables(schema, resolvedOperation?.variableDefinitions, variables, walk, collect) - return [...collected.values()].sort((a, b) => a.kind.localeCompare(b.kind) || a.name.localeCompare(b.name)) + return { + elements: [...collected.values()].sort((a, b) => a.kind.localeCompare(b.kind) || a.name.localeCompare(b.name)), + truncated: walk.truncated, + } } type Collect = (usage: DeprecatedElementUsage) => void @@ -225,9 +248,11 @@ function getPath(ancestors: readonly (ASTNode | readonly ASTNode[])[], leaf?: st return segments.join('.') } -interface WalkLimits { +interface WalkState { maxVariableDepth: number - maxVariableNodes: number + /** Remaining node budget, shared across every variable of the operation. */ + remaining: number + truncated: boolean } /** @@ -242,13 +267,11 @@ function collectFromVariables( schema: GraphQLSchema, variableDefinitions: readonly VariableDefinitionNode[] | undefined, variables: Record | null | undefined, - limits: WalkLimits, + state: WalkState, collect: Collect, ): void { if (variables == null) return - const budget = { remaining: limits.maxVariableNodes } - for (const variableDefinition of variableDefinitions ?? []) { const variableName = variableDefinition.variable.name.value if (!Object.hasOwn(variables, variableName)) continue @@ -256,21 +279,19 @@ function collectFromVariables( const type = typeFromAST(schema, variableDefinition.type) if (!isInputType(type)) continue - walkInputValue(type, variables[variableName], `$${variableName}`, 0, limits, budget, collect) + walkInputValue(type, variables[variableName], `$${variableName}`, 0, state, collect) } } -function walkInputValue( - type: GraphQLInputType, - value: unknown, - path: string, - depth: number, - limits: WalkLimits, - budget: { remaining: number }, - collect: Collect, -): void { - if (depth > limits.maxVariableDepth || budget.remaining <= 0 || value == null) return - budget.remaining-- +function walkInputValue(type: GraphQLInputType, value: unknown, path: string, depth: number, state: WalkState, collect: Collect): void { + // A limit stopping the descent is a loss of telemetry, so it is recorded; a null value is simply + // nothing to walk, and must not be reported as one. + if (depth > state.maxVariableDepth || state.remaining <= 0) { + state.truncated = true + return + } + if (value == null) return + state.remaining-- // Unwrapped inline rather than by recursing, so a nullability wrapper doesn't spend a node from // the budget for a value that has not been descended into yet. @@ -282,8 +303,11 @@ function walkInputValue( for (const [index, item] of items.entries()) { // Checked here rather than relying on the recursive call returning early: the loop itself is // the unbounded part, since list length comes from the payload and each step builds a path. - if (budget.remaining <= 0) return - walkInputValue(unwrapped.ofType, item, Array.isArray(value) ? `${path}.${index}` : path, depth + 1, limits, budget, collect) + if (state.remaining <= 0) { + state.truncated = true + return + } + walkInputValue(unwrapped.ofType, item, Array.isArray(value) ? `${path}.${index}` : path, depth + 1, state, collect) } return } @@ -302,7 +326,10 @@ function walkInputValue( if (!isInputObjectType(unwrapped) || !isRecord(value)) return for (const field of Object.values(unwrapped.getFields())) { - if (budget.remaining <= 0) return + if (state.remaining <= 0) { + state.truncated = true + return + } if (!Object.hasOwn(value, field.name)) continue const fieldPath = `${path}.${field.name}` @@ -312,7 +339,7 @@ function walkInputValue( collect({ kind: 'input-field', name: `${unwrapped.name}.${field.name}`, deprecationReason: field.deprecationReason, path: fieldPath }) } - walkInputValue(field.type, value[field.name], fieldPath, depth + 1, limits, budget, collect) + walkInputValue(field.type, value[field.name], fieldPath, depth + 1, state, collect) } } diff --git a/src/logging.ts b/src/logging.ts index 9dfc136..5715e4b 100644 --- a/src/logging.ts +++ b/src/logging.ts @@ -70,6 +70,11 @@ export interface GraphQLLogOperationInfo extend * Omitted from the log entry when empty. */ deprecatedElements?: DeprecatedElementUsage[] + /** + * Whether a limit stopped deprecated element collection, meaning `deprecatedElements` may be + * incomplete. Omitted from the log entry when false. + */ + deprecatedElementsTruncated?: boolean /** * The logger to use. */ @@ -93,6 +98,7 @@ export const logGraphQLOperation = ({ variables, result, deprecatedElements, + deprecatedElementsTruncated, logger, logLevel = 'info', ...rest @@ -111,6 +117,7 @@ export const logGraphQLOperation = ({ result: result ? omitBy(result, isNil) : undefined, isIntrospectionQuery: isIntrospection || undefined, deprecatedElements: deprecatedElements?.length ? deprecatedElements : undefined, + deprecatedElementsTruncated: deprecatedElementsTruncated || undefined, ...omitBy(rest, isNil), }, isNil, @@ -147,10 +154,14 @@ export const logSubscriptionOperation = ({ const { data, ...resultWithoutData } = result ?? {} let deprecatedElements: DeprecatedElementUsage[] | undefined + let deprecatedElementsTruncated: boolean | undefined if (includeDeprecatedElements) { try { - deprecatedElements = collectDeprecatedElementUsage({ schema, document, operationName, variables: variableValues }) + const usage = collectDeprecatedElementUsage({ schema, document, operationName, variables: variableValues }) + deprecatedElements = usage.elements + deprecatedElementsTruncated = usage.truncated } catch (error) { + // Telemetry must never take down the subscription that produced it. logger.warn('Failed to collect deprecated schema element usage', { error, operationName }) } } @@ -164,6 +175,7 @@ export const logSubscriptionOperation = ({ variables: variableValues, result: resultWithoutData, deprecatedElements, + deprecatedElementsTruncated, logger, logLevel, }) From 542a5805e262f68cc33c15138a887bc28bbf5ba7 Mon Sep 17 00:00:00 2001 From: Sam Curry Date: Tue, 18 Aug 2026 18:27:13 +0800 Subject: [PATCH 5/5] refactor(deprecation)!: drop deprecationReason and scope the default constants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review: the reason adds noise without earning it. It is static schema data, identical on every record for a given element, so it inflates every log entry to repeat something the schema already answers — and anyone acting on this telemetry has the schema in front of them. `kind` and `name` identify the element; that is what usage queries aggregate on. Also renames the exported defaults, which said nothing about what they limit once imported from the package root: DEFAULT_MAX_VARIABLE_DEPTH -> DEFAULT_DEPRECATION_MAX_VARIABLE_DEPTH DEFAULT_MAX_VARIABLE_NODES -> DEFAULT_DEPRECATION_MAX_VARIABLE_NODES DEFAULT_MAX_ELEMENTS -> DEFAULT_DEPRECATION_MAX_ELEMENTS The option names on `collectDeprecatedElementUsage` keep their short forms: the function name already scopes them. The test that asserted the reason for each kind now asserts the elements themselves, so it still covers all five kinds from one operation. BREAKING CHANGE: DeprecatedElementUsage no longer carries deprecationReason, and the three DEFAULT_MAX_* constants are renamed. Unreleased, so no published version is affected. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 2 +- src/deprecation.spec.ts | 50 +++++++++++++++-------------------------- src/deprecation.ts | 24 +++++++------------- src/logging.spec.ts | 1 - 4 files changed, 27 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index 8304fe6..adbd379 100644 --- a/README.md +++ b/README.md @@ -289,7 +289,7 @@ Reports every `@deprecated` schema element an operation uses, so you can tell wh ```ts const { elements, truncated } = collectDeprecatedElementUsage({ schema, document, operation, operationName, variables }) -// elements: [{ kind: 'input-field', name: 'WidgetFilterInput.legacyId', deprecationReason: 'Use id.', path: '$input.legacyId' }] +// elements: [{ kind: 'input-field', name: 'WidgetFilterInput.legacyId', path: '$input.legacyId' }] // truncated: false ``` diff --git a/src/deprecation.spec.ts b/src/deprecation.spec.ts index 98a1bd4..1322e97 100644 --- a/src/deprecation.spec.ts +++ b/src/deprecation.spec.ts @@ -150,9 +150,7 @@ describe('collectDeprecatedElementUsage', () => { } `) - expect(usages).toEqual([ - { kind: 'output-field', name: 'Widget.legacyName', deprecationReason: 'Use name.', path: 'widget.legacyName' }, - ]) + expect(usages).toEqual([{ kind: 'output-field', name: 'Widget.legacyName', path: 'widget.legacyName' }]) }) it('records a deprecated argument given a literal value', () => { @@ -164,7 +162,7 @@ describe('collectDeprecatedElementUsage', () => { } `) - expect(usages).toEqual([{ kind: 'argument', name: 'Query.widget(legacyId)', deprecationReason: 'Use id.', path: 'widget' }]) + expect(usages).toEqual([{ kind: 'argument', name: 'Query.widget(legacyId)', path: 'widget' }]) }) it('records a deprecated argument even when its value comes from a variable', () => { @@ -195,7 +193,7 @@ describe('collectDeprecatedElementUsage', () => { } `) - expect(usages).toEqual([{ kind: 'directive-argument', name: '@audit(legacyTag)', deprecationReason: 'Use tag.', path: 'widget' }]) + expect(usages).toEqual([{ kind: 'directive-argument', name: '@audit(legacyTag)', path: 'widget' }]) }) it('records nothing when the operation selects no deprecated element', () => { @@ -235,9 +233,7 @@ describe('collectDeprecatedElementUsage', () => { it('records an input field sent via variables', () => { const usages = collect(saveWidgetViaVariable, { variables: { input: { legacyId: 'w1' } } }) - expect(usages).toEqual([ - { kind: 'input-field', name: 'WidgetFilterInput.legacyId', deprecationReason: 'Use id.', path: '$input.legacyId' }, - ]) + expect(usages).toEqual([{ kind: 'input-field', name: 'WidgetFilterInput.legacyId', path: '$input.legacyId' }]) }) it('records an input field written inline in the document', () => { @@ -249,9 +245,7 @@ describe('collectDeprecatedElementUsage', () => { } `) - expect(usages).toEqual([ - { kind: 'input-field', name: 'WidgetFilterInput.legacyId', deprecationReason: 'Use id.', path: 'saveWidget.legacyId' }, - ]) + expect(usages).toEqual([{ kind: 'input-field', name: 'WidgetFilterInput.legacyId', path: 'saveWidget.legacyId' }]) }) it('records an input field alongside a deprecated output field on the same request', () => { @@ -290,8 +284,8 @@ describe('collectDeprecatedElementUsage', () => { }) expect(usages).toEqual([ - { kind: 'input-field', name: 'NestedInput.legacyFlag', deprecationReason: 'Use flag.', path: '$input.nested.legacyFlag' }, - { kind: 'input-field', name: 'WidgetFilterInput.legacyNested', deprecationReason: 'Use nested.', path: '$input.legacyNested' }, + { kind: 'input-field', name: 'NestedInput.legacyFlag', path: '$input.nested.legacyFlag' }, + { kind: 'input-field', name: 'WidgetFilterInput.legacyNested', path: '$input.legacyNested' }, ]) }) @@ -300,9 +294,7 @@ describe('collectDeprecatedElementUsage', () => { variables: { input: { tags: [{ label: 'kept' }, { legacyLabel: 'gone' }] } }, }) - expect(usages).toEqual([ - { kind: 'input-field', name: 'TagInput.legacyLabel', deprecationReason: 'Use label.', path: '$input.tags.1.legacyLabel' }, - ]) + expect(usages).toEqual([{ kind: 'input-field', name: 'TagInput.legacyLabel', path: '$input.tags.1.legacyLabel' }]) }) it('records one entry however many list elements carry the same field', () => { @@ -455,7 +447,7 @@ describe('collectDeprecatedElementUsage', () => { } `) - expect(usages).toEqual([{ kind: 'enum-value', name: 'WidgetStatus.LEGACY_STATUS', deprecationReason: 'Use ACTIVE.', path: 'widget' }]) + expect(usages).toEqual([{ kind: 'enum-value', name: 'WidgetStatus.LEGACY_STATUS', path: 'widget' }]) }) it('records an enum value sent as a variable', () => { @@ -470,25 +462,19 @@ describe('collectDeprecatedElementUsage', () => { { variables: { status: 'LEGACY_STATUS' } }, ) - expect(usages).toEqual([ - { kind: 'enum-value', name: 'WidgetStatus.LEGACY_STATUS', deprecationReason: 'Use ACTIVE.', path: '$status' }, - ]) + expect(usages).toEqual([{ kind: 'enum-value', name: 'WidgetStatus.LEGACY_STATUS', path: '$status' }]) }) it('records an enum value nested inside an input object sent as a variable', () => { const usages = collect(saveWidgetViaVariable, { variables: { input: { status: 'LEGACY_STATUS' } } }) - expect(usages).toEqual([ - { kind: 'enum-value', name: 'WidgetStatus.LEGACY_STATUS', deprecationReason: 'Use ACTIVE.', path: '$input.status' }, - ]) + expect(usages).toEqual([{ kind: 'enum-value', name: 'WidgetStatus.LEGACY_STATUS', path: '$input.status' }]) }) it('records an enum value inside a list, with its index in the path', () => { const usages = collect(saveWidgetViaVariable, { variables: { input: { statuses: ['ACTIVE', 'LEGACY_STATUS'] } } }) - expect(usages).toEqual([ - { kind: 'enum-value', name: 'WidgetStatus.LEGACY_STATUS', deprecationReason: 'Use ACTIVE.', path: '$input.statuses.1' }, - ]) + expect(usages).toEqual([{ kind: 'enum-value', name: 'WidgetStatus.LEGACY_STATUS', path: '$input.statuses.1' }]) }) it('records one entry however many list elements repeat the same value', () => { @@ -523,7 +509,7 @@ describe('collectDeprecatedElementUsage', () => { }) describe('result shape', () => { - it('carries the deprecation reason for every kind', () => { + it('detects all five kinds from a single operation', () => { const usages = collect( /* GraphQL */ ` mutation EveryKind($input: WidgetFilterInput!) { @@ -538,11 +524,11 @@ describe('collectDeprecatedElementUsage', () => { { variables: { input: { status: 'LEGACY_STATUS' } } }, ) - expect(usages.map(({ kind, deprecationReason }) => [kind, deprecationReason])).toEqual([ - ['directive-argument', 'Use tag.'], - ['enum-value', 'Use ACTIVE.'], - ['input-field', 'Use id.'], - ['output-field', 'Use name.'], + expect(usages).toEqual([ + { kind: 'directive-argument', name: '@audit(legacyTag)', path: 'saveWidget' }, + { kind: 'enum-value', name: 'WidgetStatus.LEGACY_STATUS', path: '$input.status' }, + { kind: 'input-field', name: 'WidgetFilterInput.legacyId', path: 'saveWidget.legacyId' }, + { kind: 'output-field', name: 'Widget.legacyName', path: 'saveWidget.legacyName' }, ]) }) diff --git a/src/deprecation.ts b/src/deprecation.ts index d6e2dd3..88730dc 100644 --- a/src/deprecation.ts +++ b/src/deprecation.ts @@ -29,18 +29,18 @@ import { * mutually recursive, and variables are walked before graphql-js coerces them — so nothing else * bounds how deep a hand-crafted payload can go. Far above any real operation's nesting. */ -export const DEFAULT_MAX_VARIABLE_DEPTH = 25 +export const DEFAULT_DEPRECATION_MAX_VARIABLE_DEPTH = 25 /** * Bounds the *breadth* of the variable walk, which the depth cap does not: a single large array of * small input objects is shallow but arbitrarily wide. */ -export const DEFAULT_MAX_VARIABLE_NODES = 10_000 +export const DEFAULT_DEPRECATION_MAX_VARIABLE_NODES = 10_000 /** * Caps the collected result so one hostile document cannot blow up a log entry. */ -export const DEFAULT_MAX_ELEMENTS = 50 +export const DEFAULT_DEPRECATION_MAX_ELEMENTS = 50 const UNKNOWN = '' @@ -53,10 +53,6 @@ export interface DeprecatedElementUsage { * `Type.field`, `Type.field(arg)`, `@directive(arg)`, `InputType.field`, `EnumType.VALUE`. */ name: string - /** - * The `reason` recorded on the element's `@deprecated` directive. - */ - deprecationReason: string /** * Best-effort location, for debugging a single record — not an aggregation key. A field selected * inside a fragment definition has no enclosing field in its ancestry, so its path is relative to @@ -122,9 +118,9 @@ export function collectDeprecatedElementUsage({ operation, operationName, variables, - maxVariableDepth = DEFAULT_MAX_VARIABLE_DEPTH, - maxVariableNodes = DEFAULT_MAX_VARIABLE_NODES, - maxElements = DEFAULT_MAX_ELEMENTS, + maxVariableDepth = DEFAULT_DEPRECATION_MAX_VARIABLE_DEPTH, + maxVariableNodes = DEFAULT_DEPRECATION_MAX_VARIABLE_NODES, + maxElements = DEFAULT_DEPRECATION_MAX_ELEMENTS, }: CollectDeprecatedElementUsageOptions): CollectDeprecatedElementUsageResult { const collected = new Map() const walk: WalkState = { maxVariableDepth, remaining: maxVariableNodes, truncated: false } @@ -181,7 +177,6 @@ function collectFromDocument(schema: GraphQLSchema, document: DocumentNode, coll collect({ kind: 'output-field', name: `${typeInfo.getParentType()?.name ?? UNKNOWN}.${field.name}`, - deprecationReason: field.deprecationReason, path: getPath(ancestors, field.name), }) }, @@ -197,7 +192,6 @@ function collectFromDocument(schema: GraphQLSchema, document: DocumentNode, coll name: directive ? `@${directive.name}(${argument.name})` : `${typeInfo.getParentType()?.name ?? UNKNOWN}.${typeInfo.getFieldDef()?.name ?? UNKNOWN}(${argument.name})`, - deprecationReason: argument.deprecationReason, path: getPath(ancestors), }) }, @@ -211,7 +205,6 @@ function collectFromDocument(schema: GraphQLSchema, document: DocumentNode, coll collect({ kind: 'input-field', name: `${parentType.name}.${inputField.name}`, - deprecationReason: inputField.deprecationReason, path: getPath(ancestors, inputField.name), }) }, @@ -223,7 +216,6 @@ function collectFromDocument(schema: GraphQLSchema, document: DocumentNode, coll collect({ kind: 'enum-value', name: `${enumType.name}.${enumValue.name}`, - deprecationReason: enumValue.deprecationReason, path: getPath(ancestors), }) }, @@ -317,7 +309,7 @@ function walkInputValue(type: GraphQLInputType, value: unknown, path: string, de if (typeof value !== 'string') return const enumValue = unwrapped.getValue(value) if (enumValue?.deprecationReason != null) { - collect({ kind: 'enum-value', name: `${unwrapped.name}.${enumValue.name}`, deprecationReason: enumValue.deprecationReason, path }) + collect({ kind: 'enum-value', name: `${unwrapped.name}.${enumValue.name}`, path }) } return } @@ -336,7 +328,7 @@ function walkInputValue(type: GraphQLInputType, value: unknown, path: string, de // Presence is the signal, explicit `null` included: a client still sending the field would // break if it were removed from the schema. if (field.deprecationReason != null) { - collect({ kind: 'input-field', name: `${unwrapped.name}.${field.name}`, deprecationReason: field.deprecationReason, path: fieldPath }) + collect({ kind: 'input-field', name: `${unwrapped.name}.${field.name}`, path: fieldPath }) } walkInputValue(field.type, value[field.name], fieldPath, depth + 1, state, collect) diff --git a/src/logging.spec.ts b/src/logging.spec.ts index 0103cfa..11e79ed 100644 --- a/src/logging.spec.ts +++ b/src/logging.spec.ts @@ -11,7 +11,6 @@ const loggedEntry = (logger: ReturnType): Record