Skip to content

AWS AppSync Support #287

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.

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

Closed
wants to merge 8 commits into from
Closed

AWS AppSync Support #287

wants to merge 8 commits into from

Conversation

bjorg
Copy link
Contributor

@bjorg bjorg commented Sep 29, 2020

The following changes allow me to use GraphQL.Client from a Blazor WebAssembly app. Please review and let me know what I need to address before submitting a pull-request.

Changes:

  1. Added WebSocketEndPoint property to allow specifying a separate websocket endpoint.
  2. Added PreprocessSubscriptionRequest to disambiguate between query and subscription pre-processing.
  3. Converted some Console.WriteLine() statements to Debug.WriteLine().
  4. Added some Debug.WriteLine() to help with debugging the AppSync protocol.
  5. Fixed an issue were WebAssembly was not properly detected (oops).

throw;
}

// TODO: wait for "connection_ack" response
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically, the client should be waiting for the connection_ack response before proceeding. For now, I put a TODO comment here as I don't have the experience with RX to know how to properly codify this step.

Copy link
Collaborator

@rose-a rose-a Oct 7, 2020

Choose a reason for hiding this comment

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

In the Apollo GraphQL over WebSocket Protocol description they write that the server may respond with GQL_CONNECTION_ACK... so it might be better to not wait for it?

But it also might resolve the unpredictable tests problem...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Urgh... may is the worst.

@bjorg
Copy link
Contributor Author

bjorg commented Sep 29, 2020

Here a sample Blazor app built with these changes: https://github.com/bjorg/GraphQlAppSyncTest/blob/main/MyApp/Pages/Index.razor

Using the new subscription pre-processor, I was able create a fairly simple adapter to configure GraphQL.Client for AppSync:
https://github.com/bjorg/GraphQlAppSyncTest/blob/main/MyApp/GraphQLHttpClientOptionsEx.cs

@bjorg
Copy link
Contributor Author

bjorg commented Oct 4, 2020

@rose-a Do you have any feedback on these changes?

@rose-a
Copy link
Collaborator

rose-a commented Oct 5, 2020

@bjorg Sorry, I did not yet find time to review your work, I hope I'll manage sometime this week...

@jgowdy
Copy link

jgowdy commented Oct 6, 2020

This would be very valuable for my team. Thanks for contributing this!

@bjorg
Copy link
Contributor Author

bjorg commented Oct 6, 2020

I've started splitting these changes up into smaller, bitesize pull-requests.

  1. Fix Blazor WASM check: fix Blazor WASM check #289
  2. Debug logging for received payloads: Debug logging for received payloads #290
  3. Fix issues with GraphQLRequest: GraphQLRequest fixes #291

Once they are approved and merged, I will update this pull-request. Thx.

@Shane32
Copy link
Member

Shane32 commented Oct 6, 2020

👍 It's always easier to review smaller PRs 👍

@MikePapinski
Copy link

as the changes has been merged, can we close this PR to do not mislead people that AWS AppSync is not working with this client?

I've started splitting these changes up into smaller, bitesize pull-requests.

  1. Fix Blazor WASM check: fix Blazor WASM check #289
  2. Debug logging for received payloads: Debug logging for received payloads #290
  3. Fix issues with GraphQLRequest: GraphQLRequest fixes #291

Once they are approved and merged, I will update this pull-request. Thx.

@rose-a
Copy link
Collaborator

rose-a commented Dec 20, 2021

Closing as beeing outdated...

@rose-a rose-a closed this Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants