Use struct-returning partial active patterns by xperiandri · Pull Request #610 · fsprojects/FSharp.Data.GraphQL

xperiandri · 2026-09-18T17:49:01Z

Summary

Converts production partial active patterns to struct-returning patterns and adds ValueOption reflection support with the narrowly required consumer adaptations.

Included

[<return: Struct>] plus ValueSome/ValueNone for eligible partial active patterns across Shared, Server, ASP.NET Core, and Client code.
New struct-returning ValueOption reflection pattern and type-matching/serialization consumers.
Complete active patterns and patterns already returning structs remain unchanged.

Review scope

This PR contains only F# active-pattern work. Optional-parameter/voption schema plumbing is in the stacked follow-up PR #604 and should not be reviewed here.

Follow-up: #604

Verification

Both solutions build with 0 warnings/errors; 655 unit tests passed with 5 pre-existing skips; 105 integration tests passed.

github-actions · 2026-09-18T17:57:52Z

Test Results

9 files 9 suites 13m 8s ⏱️
765 tests 760 ✅ 5 💤 0 ❌
2 295 runs 2 280 ✅ 15 💤 0 ❌

Results for commit cbfb9f1.

♻️ This comment has been updated with latest results.

Copilot

🟡 Changes recommended

One converted pattern retains its success-path allocation, and newly exposed public reflection APIs require documentation.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Converts partial active patterns to struct-returning patterns and adds client-side ValueOption reflection/deserialization support.

Changes:

Replaces option results with voption across active patterns.
Adds reflected construction and type matching for ValueOption.
Adapts client deserialization for ValueOption-wrapped values.

File summaries

File	Description
`src/FSharp.Data.GraphQL.Shared/TypeSystem.fs`	Converts type-reflection patterns.
`src/FSharp.Data.GraphQL.Shared/SchemaDefinitions.fs`	Converts boxed-option matching.
`src/FSharp.Data.GraphQL.Shared/Output.fs`	Converts sequence matching.
`src/FSharp.Data.GraphQL.Shared/Helpers/Reflection.fs`	Converts reflected option matching.
`src/FSharp.Data.GraphQL.Shared/AsyncVal.fs`	Converts `AsyncVal` patterns.
`src/FSharp.Data.GraphQL.Server/ReflectionHelper.fs`	Converts collection/type patterns.
`src/FSharp.Data.GraphQL.Server/ObservableExtensions.fs`	Converts cancellation matching.
`src/FSharp.Data.GraphQL.Server/Execution.fs`	Uses `ValueOption` for list detection.
`src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLWebsocketMiddleware.fs`	Converts batch-path matching.
`src/FSharp.Data.GraphQL.Client/TextConversions.fs`	Converts text-matching patterns.
`src/FSharp.Data.GraphQL.Client/Serialization.fs`	Supports `ValueOption` deserialization.
`src/FSharp.Data.GraphQL.Client/ReflectionPatterns.fs`	Adds `ValueOption` reflection utilities.

Review details

Suppressed comments (1)

src/FSharp.Data.GraphQL.Client/ReflectionPatterns.fs:101

The new public ValueOption active pattern lacks XML documentation. Document what it matches and what value it extracts, as required for public APIs in this repository.

    [<return: Struct>]
    let (|ValueOption|_|) t =
        if isValueOption t then ValueSome (ValueOption (t.GetGenericArguments().[0]))
        else ValueNone

Files reviewed: 12/12 changed files
Comments generated: 2
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot · 2026-09-18T20:35:48Z

🟡 Changes recommended

One converted pattern retains its success-path allocation, and newly exposed public reflection APIs require documentation....

Fixed in 0889325.

Copilot

🟡 Changes recommended

Boxed value options remain unsupported by outbound client serialization and variable mapping.

Get a fresh assessment by requesting another Copilot review.

Review details

Files reviewed: 12/12 changed files
Comments generated: 1
Review effort level: Balanced

Convert eligible partial active patterns ((|Name|_|)) across the production source tree to use [<return: Struct>] with ValueSome/ValueNone instead of the reference-type option (Some/None). This avoids an allocation per pattern match and is purely a return-representation change: match-arm syntax at every call site is unaffected, so no call-site adaptations were required. Converted patterns: - Client/ReflectionPatterns.fs: Option, Array, List, Seq, EnumerableValue, OptionValue, EnumValue - Client/TextConversions.fs: StringEqualsIgnoreCase, OneOfIgnoreCase - Server/ReflectionHelper.fs (Gen module): List, Array, Set, Option, Enumerable, Queryable - Server/Execution.fs: local innerListDef backing (|HasList|_|) (attribute omitted; not valid on a nested let, struct-ness inferred from the ValueSome/ValueNone body) - Server/ObservableExtensions.fs: CanceledIndependently - Server.AspNetCore/GraphQLWebsocketMiddleware.fs: BatchPath - Shared/AsyncVal.fs: Immediate, Async - Shared/Output.fs: BoxedSeq - Shared/SchemaDefinitions.fs: private Option - Shared/TypeSystem.fs (Resolve module): FSharpFunc, FSharpOption, FSharpAsync, AsyncEnumerable - Shared/Helpers/Reflection.fs: ObjectOption (now wraps the untouched optionCast via ValueOption.ofOption; optionCast itself stays an obj option-returning function since it is also used directly elsewhere) Intentionally not converted: - Server.Relay/Node.fs (GlobalId) and Server.Relay/Connections.fs (SliceInfo): already struct-returning on dev. - Shared/TypeSystem.fs Patterns module (Scalar, Object, Interface, Union, Enum, InputObject, InputCustom, SubscriptionObject, List, Nullable, NonNull, Input, Output, Leaf, Composite, Abstract) and Resolve module's BoxedSync/BoxedAsync/BoxedExpr/BoxedFilterExpr/ BoxedAsyncFilterExpr/BoxedTaskSeq: already return ValueSome/ValueNone on dev. - Client/BaseTypes.fs, Client.DesignTime/ProvidedTypesHelper.fs, Client/Serialization.fs: extracted from commit feb8f09 but their diffs were entirely driven by an unrelated option->voption introspection/schema-field change, not by active-pattern return-type conversion; out of scope for this branch. Whole-file Fantomas formatting was intentionally skipped: all touched files already fail dotnet fantomas --check on unmodified dev HEAD (pre-existing formatting drift), and running it would have produced large unrelated diffs. Hand-formatted edits match surrounding style. Validation: - dotnet build FSharp.Data.GraphQL.slnx: 0 warnings, 0 errors - dotnet build FSharp.Data.GraphQL.Integration.slnx: 0 warnings, 0 errors - dotnet test tests/FSharp.Data.GraphQL.Tests: 655 passed, 0 failed, 5 skipped (pre-existing) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Port the remaining active-pattern addition from feb8f09 that was missed by the first pass: the new (|ValueOption|_|) partial active pattern (struct-returning from the start) plus its narrowly necessary helpers, so every active pattern touched by that commit lives on this branch. - Client/ReflectionPatterns.fs: add isValueOption, private getValueOptionCases, makeValueOption, and [<return: Struct>] (|ValueOption|_|); extend isType with a ValueOption t -> t = expected arm so the new pattern is usable by isType/isNumericType/isStringType/etc. Documented makeValueOption per this repo's XML-doc convention (the source commit's comment mixed prose with a bare <see> tag). - Client/Serialization.fs: the necessary call-site adaptations to consume the new pattern - downcastNone, downcastType, isEnumType, the enum-parsing match in downcastString, getArrayValue, and the record-field-default branch in convert now also match ValueOption t alongside the existing Option t arm. Left out (unrelated to the active-pattern addition, and not needed since this branch keeps the introspection/schema-field chain on option): - All Some/None -> ValueSome/ValueNone edits to BaseTypes.fs, Client.DesignTime/ProvidedTypesHelper.fs, and the rest of Serialization.fs/BaseTypes.fs driven purely by introspection/schema fields becoming �option. - The getArrayValue List itype -> Array.map ... |> Array.toList to Seq.map ... |> Seq.toList micro-refactor bundled into the original commit - unrelated to active patterns. Validation: - dotnet build FSharp.Data.GraphQL.slnx: 0 warnings, 0 errors - dotnet build FSharp.Data.GraphQL.Integration.slnx: 0 warnings, 0 errors - dotnet test tests/FSharp.Data.GraphQL.Tests: 655 passed, 0 failed, 5 skipped (pre-existing) - dotnet test tests/FSharp.Data.GraphQL.IntegrationTests: 105 passed, 0 failed - dotnet fantomas --check on both touched files: both already fail on unmodified HEAD (pre-existing drift, confirmed via git stash), consistent with the first commit's decision to skip whole-file formatting; edits hand-formatted to match surrounding style. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

Copilot

🟢 Approval recommended

The conversions are coherent and scoped correctly, with only a non-blocking XML documentation mismatch noted.

Review details

Files reviewed: 12/12 changed files
Comments generated: 1
Review effort level: Balanced

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

This was referenced Sep 18, 2026

Fixed streaming execution and middleware behavior #612

Merged

Hardened WebSocket streaming and incremental delivery #608

Merged

xperiandri mentioned this pull request Sep 18, 2026

Use [<Struct>] ? optional parameters and voption in Define/DefineRec and the introspection chain #604

Merged

xperiandri force-pushed the dev branch from 771d526 to b954070 Compare September 18, 2026 19:30

xperiandri force-pushed the struct-active-patterns branch from 31d3b6e to bf8f8f3 Compare September 18, 2026 19:54

xperiandri requested a balanced review from Copilot September 18, 2026 20:00

Copilot started reviewing on behalf of xperiandri September 18, 2026 20:01 View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Comment thread src/FSharp.Data.GraphQL.Shared/Helpers/Reflection.fs Outdated

Comment thread src/FSharp.Data.GraphQL.Client/ReflectionPatterns.fs

Copilot started work on behalf of xperiandri September 18, 2026 20:29 View session

Copilot finished work on behalf of xperiandri September 18, 2026 20:36

xperiandri requested a balanced review from Copilot September 18, 2026 20:36

Copilot started reviewing on behalf of xperiandri September 18, 2026 20:37 View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Comment thread src/FSharp.Data.GraphQL.Client/ReflectionPatterns.fs

Copilot started work on behalf of xperiandri September 18, 2026 21:34 View session

Copilot finished work on behalf of xperiandri September 18, 2026 21:42

Copilot started work on behalf of xperiandri September 18, 2026 21:46 View session

Copilot finished work on behalf of xperiandri September 18, 2026 21:47

xperiandri and others added 4 commits September 18, 2026 23:47

Fix PR review feedback

953b46f

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

Handle boxed value options in client reflection

cbfb9f1

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

xperiandri force-pushed the struct-active-patterns branch from 2f5b33c to cbfb9f1 Compare September 18, 2026 21:47

xperiandri requested a balanced review from Copilot September 18, 2026 21:47

Copilot started reviewing on behalf of xperiandri September 18, 2026 21:48 View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Comment thread src/FSharp.Data.GraphQL.Shared/Helpers/Reflection.fs

Copilot started work on behalf of xperiandri September 18, 2026 22:04 View session

Update ObjectOption XML docs

0135497

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

Copilot finished work on behalf of xperiandri September 18, 2026 22:07

xperiandri commented Sep 18, 2026

View reviewed changes

Comment thread src/FSharp.Data.GraphQL.Shared/Helpers/Reflection.fs Outdated

Copilot started work on behalf of xperiandri September 18, 2026 22:12 View session

Use cref in ObjectOption docs

f039804

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

Copilot finished work on behalf of xperiandri September 18, 2026 22:13

xperiandri merged commit 4ee73cf into dev Sep 18, 2026
4 checks passed

xperiandri deleted the struct-active-patterns branch September 18, 2026 22:28

xperiandri mentioned this pull request Sep 19, 2026

Switched resolver and execution logic to voption #614

Merged

Navigation Menu

Use struct-returning partial active patterns - #610

Use struct-returning partial active patterns#610
xperiandri merged 6 commits into
devfrom
struct-active-patterns

xperiandri commented Sep 18, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Sep 18, 2026

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

xperiandri commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Included

Review scope

Verification

Uh oh!

github-actions Bot commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Sep 18, 2026

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xperiandri commented Sep 18, 2026 •

edited

Loading

github-actions Bot commented Sep 18, 2026 •

edited

Loading