HarperFast / HarperFast/harper
GraphQL schema parser's unknown-directive warning is inverted — typos pass silently
- 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
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