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

feat(multiple clients): add ability to define multiple clients #13

Merged
merged 9 commits into from
Feb 21, 2021

Conversation

samturrell
Copy link
Contributor

Adds the ability to define multiple clients.

Builds on top of #12 with its runtime config functionality.

@Gomah
Copy link
Owner

Gomah commented Dec 17, 2020

Thanks @samturrell, I can't review it at the moment but will jump on it as soon as I can.

Also, as many people only use one endpoint, couldn't we just inject the default graphql client to the context as $graphql & use .default if we have more than one client?

e.g:

With one client:

const countries = await $graphql.request(countriesQuery, variables);

With multiple clients:

const countries = await $graphql.default.request(countriesQuery, variables);
const planets = await $graphql.planets.request(query);

@drewbaker
Copy link
Contributor

drewbaker commented Dec 17, 2020

Great idea to have the one client and multiple client approaches.

@samturrell
Copy link
Contributor Author

@Gomah I thought about doing the same, but didn't want to make the API too confusing. Would also be a pain to upgrade in the future if users decide they need another endpoint, they would have to go through every request and update.

Happy to implement though if that's how you'd prefer it!

@samturrell
Copy link
Contributor Author

@Gomah any updates on this?

@Gomah
Copy link
Owner

Gomah commented Jan 11, 2021

@Gomah any updates on this?

Hey @samturrell, sorry I was away. I will try to have a look in the next coming weeks

@Gomah Gomah changed the base branch from master to develop February 4, 2021 06:47
lib/plugin.js Show resolved Hide resolved
Copy link
Owner

@Gomah Gomah left a comment

Choose a reason for hiding this comment

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

Hey @samturrell,

Sorry for the delay, I just added a few comments before I can merge this one!

I'll make sure the docs are up to date with your changes & adjust the types

lib/plugin.js Outdated Show resolved Hide resolved
lib/plugin.js Outdated Show resolved Hide resolved
@samturrell
Copy link
Contributor Author

@Gomah - I've amended in line with most of your comments. Let me know next steps whenever you can.

@Gomah Gomah self-requested a review February 21, 2021 23:40
Copy link
Owner

@Gomah Gomah left a comment

Choose a reason for hiding this comment

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

Thanks @samturrell! Will merge this to develop & work on the docs + types before the release!

@Gomah Gomah merged commit a480094 into Gomah:develop Feb 21, 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.

3 participants