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

Add tagged template strings to enable query string linting #155

Closed
stubailo opened this issue Apr 26, 2016 · 7 comments
Closed

Add tagged template strings to enable query string linting #155

stubailo opened this issue Apr 26, 2016 · 7 comments
Assignees
Milestone

Comments

@stubailo
Copy link
Contributor

stubailo commented Apr 26, 2016

The GraphQL ESlint plugin identifies queries via a template literal tag, gql by default.

https://github.com/apollostack/eslint-plugin-graphql

We should think about the best way to encourage people to use such a tag, and what other benefits we can get from it.

For example, we could have the template tag do the parsing, and memoize the results. We could also add a warning to the client if you don't use it, so that we can ensure all queries are linted.

Down the road, other benefits include targeting this template string with a query precompiler for performance.

@stubailo stubailo changed the title Add support for tagged template strings to work with linting Add tagged template strings to enable linting Apr 26, 2016
@stubailo stubailo changed the title Add tagged template strings to enable linting Add tagged template strings to enable query string linting Apr 26, 2016
@stubailo stubailo self-assigned this May 2, 2016
@stubailo stubailo added this to the 5/10 cycle milestone May 2, 2016
@stubailo
Copy link
Contributor Author

stubailo commented May 2, 2016

Big questions to answer:

Should we keep parsing in ApolloClient as a courtesy

If we move all parsing into the gql template tag, we can remove the dependency on the GraphQL parser from ApolloClient. However, we should consider the getting started experience - should we require that people require a separate module from the start, or should the client library do parsing as a courtesy, and simply print a warning requesting that the developer should use gql?

In short, should we require this:

import { gql } from 'apollo-client/gql';
import ApolloClient from 'apollo-client';

...

client.query({
  query: gql`...`
});

Or allow people to do:

import ApolloClient from 'apollo-client';

...

client.query({
  query: `...`
});

But just print a warning reminding them to use gql?

How to enable people to set up pre-compilation and avoid loading the parser in production

One of the biggest benefits of this approach is that we can start allowing people to precompile queries, which allows at least three great optimizations:

  1. Avoid runtime dependency on graphql, which saves load size
  2. Avoid running the parser all the time at runtime (probably a very small cost)
  3. Allow memoizing queries into a hash, and sending just the hash to the server, saving on network and query validation costs

This means that even though using gql parsing at runtime is convenient, we would want people to be able to turn it off for production if they are precompiling.

Given the limitations of current bundling systems, this probably means it needs to be in a separate module - one of the following:

// Subdirectory of client
import gql from 'apollo-client/gql';

// Separate NPM module entirely
import gql from 'gql-template-literal';

Then one could simply remove all of those imports in the production build.

Interested in feedback @martijnwalraven @jbaxleyiii @abhiaiyer91 since this would be a pretty big change in how the library is used.

@abhiaiyer91
Copy link
Contributor

One thing people that people do not like is when you take away their magic. I for one, don't like magic and err on the side of good tooling.

I do not think requiring gql is a big deal, and if having it in your workflow is the recommended approach I do not see the need to even encourage people otherwise.

I actually think its more approachable. One of the first things I looked into when diving into Apollo was, "How does this just work? This is magical". Especially comparing this to the counterparts in other Gql implementations. Having the gql template there makes the implementation more transparent.

I'd like to see the workflow of using the module in Production vs. Development and how that would be on users in terms of friction. Will people know the best practices between environments? Even with that, education can solve that.

Pretty big change, good thing its still early. Hahah if only we can eslint fix this once enabled to auto add template tags to queries.

@stubailo
Copy link
Contributor Author

stubailo commented May 2, 2016

Hahah if only we can eslint fix this once enabled to auto add template tags to queries.

hah, would be cool! at least we can print good errors - "hey, you forgot to put gql here!"

@stubailo
Copy link
Contributor Author

stubailo commented May 3, 2016

Solution at #168

@stubailo
Copy link
Contributor Author

stubailo commented May 4, 2016

Done!

@stubailo stubailo closed this as completed May 4, 2016
@deoqc
Copy link

deoqc commented May 5, 2016

The way I understand, Relay require all the parsing in the pre-compilation - so you have an optimized situation but can't do string interpolation in general 'cause it needs to be able to resolve it in the pre-compilation (so you still can do it in situations like getFragment of course).

I understood that apollo will have room for both approaches, optimized and parsing on the client, and maybe use both in production. Is this the way forward? Or won't be a effort to have parsing on the client good for a production environment, and will just be a good devtool?

@stubailo
Copy link
Contributor Author

stubailo commented May 5, 2016

Yeah right now you can use both in production! This is just one step toward making pre compilation an option if you have really specific performance needs.

jbaxleyiii pushed a commit that referenced this issue Oct 17, 2017
I believe these were wrong, but please double-check before merging @jbaxleyiii
jbaxleyiii pushed a commit that referenced this issue Oct 18, 2017
Correct reducer documentation
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 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

No branches or pull requests

3 participants