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

added native support for using apollo-link as the network layer #409

Merged
merged 2 commits into from
Oct 4, 2017

Conversation

jbaxleyiii
Copy link

This PR adjusts the fetcher portion of graphql-tools to accept a link or a fetcher. The idea is we would deprecate (potentially) using a fetcher in favor of a link stack since it makes it easier to do more complex actions and long term would make things like stitched defer, live, and more possible.

Docs link: https://github.com/apollographql/tools-docs/pull/135

Copy link
Contributor

@freiksenet freiksenet left a comment

Choose a reason for hiding this comment

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

Thanks!

const result = await fetcher({
query,
const result = await makePromise(execute(link, {
query: typeof query === 'string' ? parse(query) : query,
Copy link
Contributor

Choose a reason for hiding this comment

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

What does link accept exactly? Cause we have parsed document already above and we print it. If it accepts parsed document, then we should pass it the parsed document.

link = fetcherToLink(link as Fetcher);
}
const introspectionResult = await makePromise(execute((link as ApolloLink), {
query: typeof introspectionQuery === 'string' ? parse(introspectionQuery) : introspectionQuery,
Copy link
Contributor

Choose a reason for hiding this comment

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

We should cache parsed introspection query.

@freiksenet
Copy link
Contributor

Is depending on apollo-link a good idea? It gets our bundle size from 9KB to 15KB, that's 1.5x size increase and it is only used in one small part of the library. Is it warranted? Can we somehow get around it? I know it's a server-side package, but people did complain about bundle size already, so we need to always consider if it's worth adding more dependencies.

@freiksenet
Copy link
Contributor

Maybe I'm overthinking it.

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