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

Switch to external graphql-tag package #313

Merged
merged 3 commits into from
Jun 27, 2016
Merged

Switch to external graphql-tag package #313

merged 3 commits into from
Jun 27, 2016

Conversation

stubailo
Copy link
Contributor

@stubailo stubailo commented Jun 24, 2016

Completes #312.
Fixes #223.

@abhiaiyer91, you might be interested.

This also makes the following unnecessary:
#188 #190 @jbaxleyiii

TODO:

  • Update CHANGELOG.md with your change
  • 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

@jbaxleyiii
Copy link
Contributor

@stubailo do you think it is more efficient (weight wise) to send the parser, or the compiled documents to the client? Mainly for a larger app. I was thinking about writing a meteor build package to take .graphql files to make the importable and parse them but was unsure if it was a good move?

@stubailo
Copy link
Contributor Author

@jbaxleyiii last time I talked to Lee Byron he was thinking it might actually be more efficient to send the parser, depending on how big your queries are. The query AST is potentially large, so just a small number of queries could end up pretty heavy was his thought.

We can also experiment right? I don't think this PR changes anything about that option being available.

@jbaxleyiii
Copy link
Contributor

@stubailo I came to the same conclusion. 👍

@helfer
Copy link
Contributor

helfer commented Jun 25, 2016

I think the most efficient thing would be to just send the operationName, if that's possible.

@stubailo
Copy link
Contributor Author

@helfer sure, but that's orthogonal to this question because we need to know the query on the client even if we don't send it.

@stubailo stubailo merged commit 2e48a63 into master Jun 27, 2016
@zol zol removed the in progress label Jun 27, 2016
@stubailo stubailo deleted the graphql-tag branch September 20, 2016 03:43
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2023
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.

4 participants