HarperFast / HarperFast/harper

GraphQL schema parser's unknown-directive warning is inverted — typos pass silently

Open
#2,417 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

**Observation**

The unknown-directive warning in the GraphQL schema parser is inverted. `resources/graphql.ts:255` (end of the field-directive chain):

```ts
} else if (server.knownGraphQLDirectives.includes(directiveName)) {
console.warn(`@${directiveName} is an unknown directive, at`, directive.loc);
}
```

The condition should be `!server.knownGraphQLDirectives.includes(directiveName)`. As written it is wrong in both directions:

- A typo'd or genuinely unknown field directive (`@primarykey`, `@indxed`, `@seled`, `@alow`) produces **no warning** and silently does nothing.
- The only directives that *do* reach the warning are **known** ones used at field level that the field chain doesn't handle (`@table`, `@export`, `@sealed`, `@splitSegments`, `@replicate`) — and they're then mislabeled "unknown".

Evidence the warning has never fired for real unknowns: `unitTests/testApp/schema.graphql:16` annotates a field with `@testingUnknownDirective`, and no warning appears in any test run.

**Why it matters**

Schema directives carry indexing, sealing, and access-control semantics, so a silently ignored typo is a silent misconfiguration: a missing index, an unsealed table, or a field-level `@alow` the author believes restricts access (see #807 for `@allow`'s own enforcement gap — the other half of this parsing chain). This warning was written to catch exactly that class of mistake.

**Fix sketch**

1. Flip the condition at `resources/graphql.ts:255`.
2. Add `splitSegments` and `replicate` to `server.knownGraphQLDirectives` (`resources/graphql.ts:38-53`) — they're handled at object level (`graphql.ts:140-141`) but missing from the list.
3. Consider adding the same unknown-directive check to the object-level chain (`graphql.ts:113-150`), which currently has no warning at all.
4. The existing `@testingUnknownDirective` fixture becomes the natural assertion target for the corrected warning.

Related: #807 (`@allow` parsed but never enforced).

Contributor guide

Open the contributing guide

Research direction

Start in resources/graphql.ts:255 and review the known-directive list at lines 38-53, then inspect the object-level chain around lines 113-150. Run the relevant unit tests using unitTests/testApp/schema.graphql:16 as the assertion fixture; done means unknown field directives warn, known directives do not, and the listed object-level directives are recognized.

Written by the indexing model from the issue text.

Assessment

Tech stack
graphql, typescript
Domain
api, backend-api-design
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
75/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.