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.

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

legacy gql (graphql-ws) support, payload options on connection init #126

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

logandeancall
Copy link

@logandeancall logandeancall commented Jan 24, 2025

I'm not sure if you want to support legacy gql (graphql-ws protocol) events , if not I can maintain a fork. We're currently using Hasura, a graphql layer, which only uses the legacy gql protocol so this is a necessity for us.

This PR also includes the option to allow params on the connection_init event. This is necessary when needing to set an authorization header. Example:

#[derive(Serialize)]
struct AuthHeaders {
    #[serde(rename = "Authorization")]
    authorization: String,
}

#[derive(Serialize)]
struct AuthPayload {
    headers: AuthHeaders,
}

    let auth_payload = serde_json::to_value(&AuthPayload {
        headers: AuthHeaders {
            authorization: format!("Bearer {}", token),
        },
    })?;

    let (client, actor) = Client::build(connection, Some(auth_payload)).await?;

This is a breaking change in the sense that anyone using Client::build will now need to pass an optional payload as a second param. I'm fairly new to rust so if there is a more idiomatic way to do this w/out causing a breaking change lmk!

@obmarg
Copy link
Owner

obmarg commented Jan 24, 2025

Hi @logandeancall - thanks for the contribution.

I'm not sure about support for the legacy protocol. I have previously resisted adding support for it, although if it's all hasura support I could be convinced to add it. Let me have a think.

The change to the build method is definitely not something I want to accept though. The non breaking way to make this change would be to add a payload method to the ClientBuilder (which Client::build returns). Im pretty sure this function already exists though.

If you're able to revert the change to build I can take a look at the rest of the change in isolation when I get a minute

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.

2 participants