Skip to content

Conversation

@toadkicker
Copy link
Contributor

This adds context.options.apolloClient to the available options so that developers can instantiate their own client and pass it to the plugin.

@phortx
Copy link
Collaborator

phortx commented Apr 27, 2019

Thank you very much for your contribution! :)

I just have three things, we should change:

  1. We then can drop the options.link option, because it's part of the client.
  2. The client should be setup within a function, because to properly setup a client, you need the context. Like this:
const options = {
  apolloClient: (context) => {
    return ...;
  }
};
  1. We then should describe in the docs how one would setup a client the same way the default client is setup.

@toadkicker
Copy link
Contributor Author

What if we change this to adding a clients/DefaultClient class, similar to how you approached the adapter problem?

@phortx
Copy link
Collaborator

phortx commented Apr 28, 2019

This would force the user to write a wrapper class around the ApolloClient, which is too much here I think :)

@toadkicker
Copy link
Contributor Author

This is how I'm setting it up in store/index, I'm not completely clear what you mean with passing context in the option:

import VuexORM from '@vuex-orm/core'
import AWSAppSyncClient from 'aws-appsync'
import { Auth } from 'aws-amplify'
import VuexORMGraphQL from '@vuex-orm/plugin-graphql'

import database from '../database'
import awsexports from '../aws-exports'

const options = {
  defaultOptions: {
    watchQuery: {
      fetchPolicy: 'cache-and-network'
    }
  },
  connectionQueryMode: 'nodes',
  database: database,
  url: awsexports.aws_appsync_graphqlEndpoint,
  includeExtensions: true,
  debug: process.env.NODE_ENV !== 'production'
}

const config = {
  url: awsexports.aws_appsync_graphqlEndpoint,
  region: awsexports.aws_appsync_region,
  auth: {
    type: awsexports.aws_appsync_authenticationType,
    jwtToken: async () => (await Auth.currentSession()).getIdToken().getJwtToken()
  }
}

const client = new AWSAppSyncClient(config, options)

options.apolloClient = client

VuexORM.use(VuexORMGraphQL, options)

export const plugins = [
  VuexORM.install(database)
]

@phortx
Copy link
Collaborator

phortx commented Apr 29, 2019

I'm not completely clear what you mean with passing context in the option:

Oh sorry, I think I didn't make that clear. In your implementation we pass the apollo client into the options as a instance of the client. But to properly setup a client, the developer could need the context. So I want to provide it. We can achieve that by changing how plugin-graphql accepts the client: not as a object, but by calling a function.

So changing your code how to setup a client it would work like this:

import VuexORM from '@vuex-orm/core'
import AWSAppSyncClient from 'aws-appsync'
import { Auth } from 'aws-amplify'
import VuexORMGraphQL from '@vuex-orm/plugin-graphql'

import database from '../database'
import awsexports from '../aws-exports'

const options = {
  defaultOptions: {
    watchQuery: {
      fetchPolicy: 'cache-and-network'
    }
  },
  connectionQueryMode: 'nodes',
  database: database,
  url: awsexports.aws_appsync_graphqlEndpoint,
  includeExtensions: true,
  debug: process.env.NODE_ENV !== 'production'
}

// I changed this!
options.apolloClient =(contex) => {
    const config = {
    url: awsexports.aws_appsync_graphqlEndpoint,
    region: awsexports.aws_appsync_region,
    auth: {
      type: awsexports.aws_appsync_authenticationType,
      jwtToken: async () => (await Auth.currentSession()).getIdToken().getJwtToken()
    }
  };

  return new AWSAppSyncClient(config, options);
};

VuexORM.use(VuexORMGraphQL, options)

export const plugins = [
  VuexORM.install(database)
]

Also I think we then can drop the support for options.link completly

…lo_client_option

# Conflicts:
#	dist/vuex-orm-graphql.es5.js
#	dist/vuex-orm-graphql.es5.js.map
#	dist/vuex-orm-graphql.umd.js
#	dist/vuex-orm-graphql.umd.js.map
#	dist/vuex-orm-graphql.es5.js
#	dist/vuex-orm-graphql.es5.js.map
#	dist/vuex-orm-graphql.umd.js
#	dist/vuex-orm-graphql.umd.js.map
@toadkicker
Copy link
Contributor Author

@phortx 👍 merge ready

@phortx
Copy link
Collaborator

phortx commented May 31, 2019

Looks good :) Thank you very much for your contribution!

@phortx phortx merged commit a292b7d into vuex-orm:master May 31, 2019
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