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

Introduce "graph" addon #189

Closed
Kasea opened this issue Jun 14, 2023 · 2 comments
Closed

Introduce "graph" addon #189

Kasea opened this issue Jun 14, 2023 · 2 comments
Labels

Comments

@Kasea
Copy link

Kasea commented Jun 14, 2023

Title says it all, think it's an addon that could either be very complex or very simple depending on what "scope" is deemed the best. I made a very simple version for work that I will attach below, but I would say it has some issues which I will discuss as well; which (if any) direction the graph addon ends up going I don't mind, think this library has a history of introducing good apis!

Code:

import type { Wretch, WretchAddon, WretchResponseChain } from "wretch";

export interface GraphAddon {
  /**
   * Executes a graph request
   *
   * @param query - The graph query
   * @param variables - Variables for the graph query
   */
  graph<T extends GraphAddon, C, R>(
    this: T & Wretch<T, C, R>,
    query: string,
    variables?: object
  ): R extends undefined ? C & WretchResponseChain<T, C, R> : R;
}

/**
 * Adds the ability to make a graph request.
 *
 */
const wretchGraph: WretchAddon<GraphAddon> = {
  wretch: {
    graph(query, variables = {}) {
      return this.post({ query, variables });
    },
  },
};

export default wretchGraph;

Issues:

  • removes the possibility of passing "url" to the post request

Almost issues:

  • should do post().json<type>() where it gets type passed from caller (should it add the data property itself or the caller?)

Potential improvements:

  • Support graph error handling
  • Change call signature to graph(opts: { query: string, variables?: object }, url?: string) ?
  • Support gql ; not 100% sure if it needs specific support and/or if out of scope.
@elbywan
Copy link
Owner

elbywan commented Jun 17, 2023

Hey @Kasea,

It has been a while since I have used graphql so apologies if I am mistaken, but sending / receiving graphql messages seem pretty basic and there two ways of hosting an HTTP graphql server - either a GET route with an url encoded query parameter or a POST route having a a json payload body.

Furthermore there are existing graphql clients that you can use on the frontend side which already do the work pretty well (apollo, relay).

So I do not really see the value in writing a dedicated addon for that.

@Kasea
Copy link
Author

Kasea commented Jun 17, 2023

Yeah graphql is very basic, haven't seen anyone using GET for graph in production ever, so I wouldn't really worry about it.

There exists a lot of graphql clients yeah, but as you've stated most of them are targeted for frontend which doesn't really give a lot of options for backend but there's also the issue of adding a lot of extra bloat for no reason (since graph is a very simple format).

I wouldn't expect a full blown out "perfect" graph implementation, the code I've supplied I'd argue would be the mvp for it, anything beyond that is just quality of life.

@elbywan elbywan closed this as not planned Won't fix, can't repro, duplicate, stale Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants