ardatan / ardatan/graphql-tools

mapSchema carries over schema validation state

Open
#4,087 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
5.4k
Forks
830
Avg merge
10h 59m
Merged PRs (30d)
45

Description

**Describe the bug**

A schema that is explicitly assumed to be validated using the `assumeValid` flag or a schema that has actually been validated is marked as such using an internal flag.

mapSchema carries over this flag to the new schema. This is debatable in the former case (when the schema is explicitly marked as assumeValid, presumably to ensure that the validation step is skipped), but is definitely incorrect in the latter scenario.

**To Reproduce**

This was surfaced in #4066 where a schema will fail validation when incompletely pruned, but only fails when using graphql-executor. This is presumably because when using graphql-executor, we get a different order of test execution, and the propertySchema has not been validated elsewhere. If you step through the test code line by line, just running the failing test, the test failed even when using the graphql-js execute function!

**Expected behavior**

This flag should not be carried over automatically. This would be the expected behavior when the flag is not set explicitly. The flag is set explicitly in cases where the schema creator wants to skip validation altogether. Whether validation should also be skipped for any schemas mapped for the original schema is unclear, and could differ in each case. This should probably be an argument for the executor, and should be raised as such in graphql-js.

Contributor guide

Open the contributing guide

Research direction

Start at mapSchema and reproduce the validation-order problem described in #4066, comparing graphql-executor with graphql-js. Trace how the validation state is transferred to the mapped schema and determine the intended handling of explicitly set assumeValid state. Done means the state is not carried over automatically and the incompletely pruned schema has regression coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend-api-design
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.