Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the problem but we should still be able to pass options and build a client if not specified. It would be better to introduce a new clientOptions
parameter. If client
is specified, use it, otherwise build one with the clientOptions
@@ -34,6 +34,7 @@ const getOptions = (options, aorFetchType, resource) => { | |||
|
|||
export default async options => { | |||
const { | |||
clientObject, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather name this one client
and rename the old client
to clientOptions
. We could then remove completely the check for ApolloClient at L48.
This will be a breaking change though and we need to update the documentation accordingly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I will change it.
But what about tests?
If we need to chage tests I am going to take 1 or 2 days
clientOptions && clientOptions instanceof ApolloClient ? clientOptions : buildApolloClient(clientOptions); | ||
let client = clientObject | ||
if(!client){ | ||
client = clientOptions && clientOptions instanceof ApolloClient ? clientOptions : buildApolloClient(clientOptions); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove that and just build the ApolloClient if no client was provided already ?
@djhi I cahnged |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there, you should also mention in the documentation that you can pass an existing client with the client
option
It is already mentioned here. https://github.com/marmelab/aor-graphql/blame/master/packages/aor-graphql-client/README.md#L103 |
Trying to use
apollo-client
2 .Easiest way is pass apollo-client v2 object directly. But this check skips client and builds v1 client.
So, If this v1 client instanceOf check is removed we can use v2 client like this.