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

require graphql import from gatsby to use #6002

Merged
merged 5 commits into from
Jun 20, 2018
Merged

require graphql import from gatsby to use #6002

merged 5 commits into from
Jun 20, 2018

Conversation

jquense
Copy link
Contributor

@jquense jquense commented Jun 18, 2018

This is a pretty big breaking change, and I can walk back some of it so it can be made compatible (like a config setting to require the import).

I need to make a gatsby site that works with relay/apollo on the client in the very near future but that isn’t possible at the moment because gatsby assumes all graphql tags are its own.

@jquense
Copy link
Contributor Author

jquense commented Jun 18, 2018

As a less severe change, we could also assume global i.e. unreferenced graphql tags are gatsby's and leave any alone that are referenced (like from 'relay' or apollo packages)

@KyleAMathews
Copy link
Contributor

I need to make a gatsby site that works with relay/apollo on the client in the very near future

💯 to this so let's make this possible.

Perhaps we deprecate auto-supporting the graphql tag in v2 and then require it in v3?

@KyleAMathews
Copy link
Contributor

Also 💯 to making the graphql tag seem less magical (even though it still will be under the hood).

@jquense
Copy link
Contributor Author

jquense commented Jun 18, 2018

I can't seem to get a local test up running using this...lets see if netlify works nothing should break

@jquense
Copy link
Contributor Author

jquense commented Jun 19, 2018

w/y/t ?

@pieh
Copy link
Contributor

pieh commented Jun 20, 2018

I like it! For this maybe we could do some codemod-esque utility to auto update people's code (maybe not print out deprecation warning until we have this tool)? Less magic that seems to come from nowehere is always good

@jquense
Copy link
Contributor Author

jquense commented Jun 20, 2018

adding some robustness to this!

@jquense
Copy link
Contributor Author

jquense commented Jun 20, 2018

ok this covers a lot more edge cases, I wanted to write the codemod as well but ran out of time...

@KyleAMathews KyleAMathews merged commit bda1929 into v2 Jun 20, 2018
@KyleAMathews
Copy link
Contributor

Looks great! Thanks for writing this! A number of people have complained about not being able to use Relay & Gatsby together and I think everyone has complained about graphql being an unimported global 😅

@KyleAMathews
Copy link
Contributor

I'll create a follow-up issue about updating the tutorial to account for this.

Once we have a codemod, we can run that on the example sites to update them.

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.

3 participants