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

Allow dynamic schema resolution #447

Merged
merged 5 commits into from
Jul 27, 2018

Conversation

nirinchev
Copy link
Contributor

@nirinchev nirinchev commented Jul 17, 2018

Sometimes, the GraphQL schema needs to be dynamically resolved (e.g. when exposing a database interface). This change allows the schema to be provided either as a static value or as a function of the connection context:

const subscriptionServer = new SubscriptionServer(
{
    schema: undefined,
    onOperation: async (message, params, socket) => {
        if (await something()) {
            params.schema = schema1;
        } else {
            params.schema = schema2;
        }

        return params;
    },
    ...
}

There are no tests for it since it's a fairly trivial change and I wasn't sure if it would be accepted, but I can write some smoke tests if required.

TODO:

  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass
  • Update CHANGELOG.md with your change

@apollo-cla
Copy link

@nirinchev: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@nirinchev
Copy link
Contributor Author

CI seems to be failing with a test that passes locally - I imagine it's possible that this change triggers an underlying race condition with that test. I'll investigate.

@nirinchev
Copy link
Contributor Author

Can I get some guidance on the direction this PR takes and whether it's something you'd like to merge? I'd be happy to add tests and update the changelog as soon as I get some confirmation that this change is consistent with the direction you'd like to take with this library.

@NeoPhi
Copy link
Contributor

NeoPhi commented Jul 23, 2018

I think an approach along the lines of apollo-server, would be for the onOperation result to optionally return the schema that should be used as it already handles returning the context, etc.

@nirinchev
Copy link
Contributor Author

@NeoPhi thanks for the feedback - can you review if the current implementation is aligned with what you had in mind?

@NeoPhi
Copy link
Contributor

NeoPhi commented Jul 26, 2018

This is looking great. We will also want updates for the README and docs.

@nirinchev
Copy link
Contributor Author

I updated the readme and the docs - they don't seem to be too verbose, so wasn't sure into how much detail I should go.

@NeoPhi NeoPhi merged commit 40c3c3c into apollographql:master Jul 27, 2018
@NeoPhi
Copy link
Contributor

NeoPhi commented Jul 27, 2018

Thank you very much!

@nirinchev
Copy link
Contributor Author

You're welcome! Do you guys do releases based on fixed time periods or when a certain number of features/bug fixes accumulate? Just wondering what timeframe to give our customers about this making it into an official release 🙂

@NeoPhi
Copy link
Contributor

NeoPhi commented Jul 27, 2018

I've been trying to do continuous releases. As such https://github.com/apollographql/subscriptions-transport-ws/releases/tag/v0.9.14 just published.

bikov added a commit to bikov/subscriptions-transport-ws that referenced this pull request Nov 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants