Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

Expand GraphqlParams allowed record types #124

Closed
2 tasks done
rdunk opened this issue Mar 5, 2021 · 4 comments
Closed
2 tasks done

Expand GraphqlParams allowed record types #124

rdunk opened this issue Mar 5, 2021 · 4 comments

Comments

@rdunk
Copy link
Contributor

rdunk commented Mar 5, 2021

Overview

I saw that a PR is currently in progress for updating documentation related to the GraphQL client, so thought I'd flag here.

The GraphqlParams type defines an optional query property with type Record<string, string | number>. In other words, query parameters must be an object with values of either string or number.

Some mutations require passing an object as a parameter value. For example eventBridgeWebhookSubscriptionCreate requires a webhookSubscription parameter of type EventBridgeWebhookSubscriptionInput. Attempting to pass a value like that results in type errors.

I'm not completely familiar with all queries/mutations offered by the Admin API GraphQL endpoint and their required parameters, so I'm not sure what this type should be.

Type

  • Changes to existing features

Motivation

I'm attempting to use the GraphQL client provided by this library to create EventBridge subscriptions, but I'm seeing type errors. Apologies if I've misunderstood how the GraphQL client works, I understand documentation is still in progress. Thanks!

Checklist

  • I have described this feature request in a way that is actionable (if possible)
@rdunk
Copy link
Contributor Author

rdunk commented Mar 5, 2021

Looking at the code a bit more: I've realised EventBridge subscriptions are probably better handled via Shopify.Webhooks. Still interested in discussing though!

@thecodepixi
Copy link
Contributor

Hey! Yeah, you're correct, the EventBridge subscriptions should be handled by Shopify.Webhooks, but this is a good sign that that might also need better documentation. I'm working on improved GraphQL and REST client documentation now, but we can definitely take another look at our documentation around webhooks.

@rdunk
Copy link
Contributor Author

rdunk commented Mar 5, 2021

Thanks for the quick response, and for working on the docs!

FWIW I thought the current docs on webhooks were helpful, I just hadn't realised I could specify the deliveryMethod to register webhooks specifically for EventBridge, which is why I ventured down the route of initialising a GraphQL client instance instead. When I inspected the types it became obvious though, TS is great.

@thecodepixi
Copy link
Contributor

Hey @rdunk, I'm going to go ahead and close this since you should be good to go with Shopify.Webhooks and we're working on the enhanced docs, but if you run into any more issues with that or with the clients, feel free to open another issue!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants