Bump version to 3.1.1 and add tests for reserved scalar names handling#550
Bump version to 3.1.1 and add tests for reserved scalar names handling#550fwaris wants to merge 2 commits intofsprojects:devfrom
Conversation
xperiandri
left a comment
xperiandri
left a comment
There was a problem hiding this comment.
Looks good, just a few minor changes.
Thank you very much for the contribution!
RELEASE_NOTES.md
Outdated
| ### 3.1.1 - 2026-04-03 | ||
| * Fixed GraphQL client provider handling for schema types that reuse reserved scalar names such as `Date`, so introspection kind now takes precedence over built-in scalar mappings. | ||
|
|
There was a problem hiding this comment.
this must go to the bottom of the file
| | None when isScalar typeName -> struct (variableName, typeName, typeof<string option>) | ||
| | None when TypeMapping.isScalarTypeName schemaTypes typeName -> struct (variableName, typeName, typeof<string option>) | ||
| | None -> | ||
| match schemaProvidedTypes.TryFind(typeName) with |
There was a problem hiding this comment.
| match schemaProvidedTypes.TryFind(typeName) with | |
| match schemaProvidedTypes.TryFind typeName with |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the GraphQL client provider to prefer introspection type kinds over built-in scalar mappings when names conflict (e.g., schema defines Date as an object/input object), and adds regression coverage plus a version bump.
Changes:
- Adjust scalar/type-name resolution so schema type kinds take precedence over reserved/built-in scalar name mappings.
- Add integration tests and introspection fixtures covering
Dateas OBJECT and INPUT_OBJECT. - Bump package version to 3.1.1 and update release notes.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/FSharp.Data.GraphQL.IntegrationTests/reserved_scalar_object_date_introspection.json | Adds fixture where Date is an OBJECT in the schema |
| tests/FSharp.Data.GraphQL.IntegrationTests/reserved_scalar_input_date_introspection.json | Adds fixture where Date is an INPUT_OBJECT in the schema |
| tests/FSharp.Data.GraphQL.IntegrationTests/ReservedScalarNameProviderTests.fs | Adds regression tests to ensure providers compile with reserved scalar name reuse |
| tests/FSharp.Data.GraphQL.IntegrationTests/FSharp.Data.GraphQL.IntegrationTests.fsproj | Includes new test file and introspection fixtures in the test project |
| src/FSharp.Data.GraphQL.Client/BaseTypes.fs | Introduces helpers to distinguish built-in scalars vs schema-defined scalars and adjusts schema type filtering |
| src/FSharp.Data.GraphQL.Client.DesignTime/ProvidedTypesHelper.fs | Switches operation variable type mapping to use the new scalar/type-name helpers |
| RELEASE_NOTES.md | Adds 3.1.1 release entry, but currently duplicates the 3.1.1 heading |
| Directory.Build.props | Bumps NuGet package version to 3.1.1 |
RELEASE_NOTES.md
Outdated
| @@ -1,3 +1,6 @@ | |||
| ### 3.1.1 - 2026-04-03 | |||
There was a problem hiding this comment.
The release notes now contain two separate sections for version 3.1.1 (one dated and one 'Unreleased'), which is confusing and can lead to incorrect changelog generation. Please merge these entries under a single 3.1.1 section, or bump the 'Unreleased' header to the next version (e.g., 3.1.2 - Unreleased) so each version appears only once.
| ### 3.1.1 - 2026-04-03 | |
| ### 3.1.2 - Unreleased |
RELEASE_NOTES.md
Outdated
| @@ -242,4 +245,4 @@ | |||
|
|
|||
| ### 3.1.1 - Unreleased | |||
There was a problem hiding this comment.
The release notes now contain two separate sections for version 3.1.1 (one dated and one 'Unreleased'), which is confusing and can lead to incorrect changelog generation. Please merge these entries under a single 3.1.1 section, or bump the 'Unreleased' header to the next version (e.g., 3.1.2 - Unreleased) so each version appears only once.
| ### 3.1.1 - Unreleased | |
| ### 3.1.2 - Unreleased |
| true |> equals true | ||
|
|
||
| [<Fact>] | ||
| let ``Should allow input object types that reuse reserved scalar names`` () = | ||
| InputDateSchema.compileSmoke () | ||
| true |> equals true |
There was a problem hiding this comment.
These assertions are no-ops (they always compare true to true) and don’t add signal beyond ‘test did not throw’. Since xUnit will already fail the test on any exception from compileSmoke, consider removing the true |> equals true lines (or replace with an assertion that reflects an observable outcome if you want runtime validation).
| true |> equals true | |
| [<Fact>] | |
| let ``Should allow input object types that reuse reserved scalar names`` () = | |
| InputDateSchema.compileSmoke () | |
| true |> equals true | |
| [<Fact>] | |
| let ``Should allow input object types that reuse reserved scalar names`` () = | |
| InputDateSchema.compileSmoke () |
|
@fwaris could you fix and I will release? |
|
addressed comments. thanks. |
xperiandri
left a comment
xperiandri
left a comment
There was a problem hiding this comment.
Both changes will be released
| * Fixed GraphQL client provider handling for schema types that reuse reserved scalar names such as `Date`, so introspection kind now takes precedence over built-in scalar mappings. | ||
|
|
||
| ### 3.1.2 - Unreleased | ||
| * Fixed planning phase crash when inline fragments reference types not included in union or interface definitions |
There was a problem hiding this comment.
| * Fixed GraphQL client provider handling for schema types that reuse reserved scalar names such as `Date`, so introspection kind now takes precedence over built-in scalar mappings. | |
| ### 3.1.2 - Unreleased | |
| * Fixed planning phase crash when inline fragments reference types not included in union or interface definitions | |
| * Fixed planning phase crash when inline fragments reference types not included in union or interface definitions | |
| * Fixed GraphQL client provider handling for schema types that reuse reserved scalar names such as `Date`, so introspection kind now takes precedence over built-in scalar mappings. |
Ran into GQL that defines 'Date' as object with two fields.
This is legal GQL but 'Date' is hardcoded to the cli type in the provider.
This PR overrides that restirction, i.e. if there is a conflict with the schema for a type name that is mapped to a known scalar, the schema overrides.
Added new tests. All tests pass.