Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

FSharp.Data.GraphQL.Server.AspNetCore #430

Merged
merged 102 commits into from
Mar 23, 2024
Merged

FSharp.Data.GraphQL.Server.AspNetCore #430

merged 102 commits into from
Mar 23, 2024

Conversation

valbers
Copy link
Collaborator

@valbers valbers commented Mar 4, 2023

These changes introduce utility code for configuring usage of FSharp.Data.GraphQL in an ASP.NET app, especially one using Giraffe, AND offer an ASP.NET middleware that implements the graphql-transport-ws Websocket subprotocol for GraphQL subscriptions (a.k.a Apollo GraphQL subscription protocol).
A companion sample chat application (the backend part) is also introduced with these changes.
This development departed from the "start-wars-api" sample.

valbers added 2 commits March 4, 2023 11:46
@valbers
ignoring the whole .vscode/ folder
45a7d48
@valbers
introducing FSharp.Data.GraphQL.Server.AppInfrastructure
c3f1a48 
These changes introduce utility code for configuring usage of
FSharp.Data.GraphQL in an ASP.NET app, especially one using Giraffe,
AND offers an ASP.NET middleware that implements the
graphql-transport-ws Websocket subprotocol for GraphQL subscriptions
(a.k.a Apollo GraphQL subscription protocol).
A companion sample chat application (the backend part) is also introduced with these
changes.
This development departed from the "start-wars-api" sample.

- Originally developed in https://github.com/valbers/graphql-transport-ws.fsharp
- See also: fsprojects#428
xperiandri
xperiandri requested changes Mar 4, 2023
View reviewed changes
Copy link
Collaborator

@xperiandri xperiandri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@valbers brlian job!
Some remarks from me.
I propose:

  1. You change stuff that is not realted to IGQLError
  2. We merge IGQLError
  3. You rebase and align your PR on top of IGQLError

.gitignore Show resolved Hide resolved
FSharp.Data.GraphQL.sln Outdated Show resolved Hide resolved
build.fsx Outdated Show resolved Hide resolved
samples/chat-app/client/TestData/schema-snapshot.json Outdated Show resolved Hide resolved
samples/chat-app/client/TestData/subscription-example.json Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server.AppInfrastructure/GraphQLWebsocketMiddleware.fs Outdated Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server.AppInfrastructure/GraphQLWebsocketMiddleware.fs Outdated Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server.AppInfrastructure/GraphQLWebsocketMiddleware.fs Outdated Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server.AppInfrastructure/Rop/Rop.fs Outdated Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server.AppInfrastructure/Serialization/GraphQLQueryDecoding.fs Outdated Show resolved Hide resolved
@xperiandri
Copy link
Collaborator

@valbers add yourself as a contributor to Directory.Build.props

@valbers
Copy link
Collaborator Author

valbers commented Mar 5, 2023

@xperiandri thank you so much for the encouraging words and taking the time to review my changes so quick and so thoroughly. I'm going to apply some of the changes you suggested and then I'll see how I can fix the build. It seems to be an inconsistent behavior of the F# compiler relating to the usage of task computation expressions.

@xperiandri
Copy link
Collaborator

You cannot have too many nesting matches inside the task.
You must wrap each match case into it to fix such a problem.
Usually changing top-level match in this way helps.

@valbers
Copy link
Collaborator Author

valbers commented Mar 7, 2023

Just so you know: I'm planning to solve all issues by the end of this week (i.e. until 2023-03-12).

@xperiandri
Copy link
Collaborator

Sounds great!

valbers and others added 5 commits March 11, 2023 19:59
@valbers
fixing HttpHandler: the "errors" property shouldn't be there if no errs
925873f 
  I know this code will be removed later because of upcoming changes
from another branch, but it's breaking the integration tests
(more specifically: the type provider). It just feels better to have it
work correctly for the moment (and have a passing build :) ).
@valbers
automatically updating chat-app/client/TestData/schema-snapshot.json
6fcbf71
@valbers @xperiandri
Apply commit suggestion
d72500a 
- use structural logging

Co-authored-by: Andrii Chebukin <xperiandri@live.ru>
@valbers @xperiandri
Apply review suggestion
5226c16 
- using `|> Task.WaitAll` instead of `|> Async.AwaitTask |> Async.RunSynchronously`

Co-authored-by: Andrii Chebukin <xperiandri@live.ru>
@valbers
trying to use more structured logging (and fixing code)
13c495e
xperiandri
xperiandri reviewed Mar 12, 2023
View reviewed changes
build.fsx Outdated Show resolved Hide resolved
build.fsx Outdated Show resolved Hide resolved
@valbers
not calling .ToString() on objects that go into structured logging
f3e85cc 
  According to suggestion during code review.
xperiandri
xperiandri requested changes Mar 12, 2023
View reviewed changes
src/FSharp.Data.GraphQL.Server.AppInfrastructure/GraphQLWebsocketMiddleware.fs Outdated Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server.AppInfrastructure/GraphQLWebsocketMiddleware.fs Outdated Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server.AppInfrastructure/GraphQLWebsocketMiddleware.fs Outdated Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server.AppInfrastructure/GraphQLWebsocketMiddleware.fs Outdated Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server.AppInfrastructure/GraphQLWebsocketMiddleware.fs Outdated Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server.AppInfrastructure/GraphQLWebsocketMiddleware.fs Outdated Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server.AppInfrastructure/GraphQLWebsocketMiddleware.fs Outdated Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server.AppInfrastructure/GraphQLWebsocketMiddleware.fs Outdated Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server.AppInfrastructure/GraphQLWebsocketMiddleware.fs Outdated Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server.AppInfrastructure/GraphQLWebsocketMiddleware.fs Outdated Show resolved Hide resolved
@xperiandri xperiandri force-pushed the introduce-graphql-transport-ws branch from 1e333c5 to 17b251e Compare March 17, 2024 20:20
@xperiandri xperiandri force-pushed the introduce-graphql-transport-ws branch from 17b251e to 38fed3c Compare March 17, 2024 20:23
@valbers valbers force-pushed the introduce-graphql-transport-ws branch from 46f53ec to cd3b1ce Compare March 17, 2024 21:46
@xperiandri xperiandri changed the title Introduce FSharp.Data.GraphQL.Server.AppInfrastructure (~= ASP.NET DI + Giraffe HttpHandler + Websocket) FSharp.Data.GraphQL.Server.AspNetCore (ASP.NET Core DI + Giraffe HttpHandler + WebSocket support) Mar 23, 2024
@xperiandri xperiandri changed the title FSharp.Data.GraphQL.Server.AspNetCore (ASP.NET Core DI + Giraffe HttpHandler + WebSocket support) FSharp.Data.GraphQL.Server.AspNetCore Mar 23, 2024
@xperiandri xperiandri merged commit 2690c19 into fsprojects:dev Mar 23, 2024
3 checks passed
xperiandri added a commit that referenced this pull request Mar 23, 2024
@valbers @xperiandri
FSharp.Data.GraphQL.Server.AspNetCore (#430)
450abf4 
Adds ASP.NET Core DI integration using a Giraffe HttpHandler and a WebSocket protocol support

Co-authored-by: Andrii Chebukin <XperiAndri@Outlook.com>
Co-authored-by: Andrii Chebukin <andrii@lula.is>
xperiandri added a commit that referenced this pull request Mar 24, 2024
@xperiandri
fixup! FSharp.Data.GraphQL.Server.AspNetCore (#430)
dc438d7
xperiandri added a commit that referenced this pull request Mar 24, 2024
@xperiandri
fixup! FSharp.Data.GraphQL.Server.AspNetCore (#430)
adcd581
xperiandri added a commit that referenced this pull request Mar 24, 2024
@xperiandri
fixup! FSharp.Data.GraphQL.Server.AspNetCore (#430)
f758970
xperiandri added a commit that referenced this pull request Mar 24, 2024
@valbers @xperiandri
FSharp.Data.GraphQL.Server.AspNetCore (#430)
9a2a107 
Adds ASP.NET Core DI integration using a Giraffe HttpHandler and a WebSocket protocol support

Co-authored-by: Andrii Chebukin <XperiAndri@Outlook.com>
Co-authored-by: Andrii Chebukin <andrii@lula.is>
xperiandri added a commit that referenced this pull request Mar 24, 2024
@xperiandri
fixup! FSharp.Data.GraphQL.Server.AspNetCore (#430)
0dd4372
xperiandri added a commit that referenced this pull request Mar 24, 2024
@valbers @xperiandri
FSharp.Data.GraphQL.Server.AspNetCore (#430)
2c86a71 
Adds ASP.NET Core DI integration using a Giraffe HttpHandler and a WebSocket protocol support

Co-authored-by: Andrii Chebukin <XperiAndri@Outlook.com>
Co-authored-by: Andrii Chebukin <andrii@lula.is>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mickhansen mickhansen mickhansen left review comments

@ivelten ivelten ivelten left review comments

@xperiandri xperiandri xperiandri approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@valbers @xperiandri @mickhansen @ivelten