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

Queries from unused themes cause "gatsby develop" to fail #16338

Closed
johno opened this issue Aug 2, 2019 · 16 comments
Closed

Queries from unused themes cause "gatsby develop" to fail #16338

johno opened this issue Aug 2, 2019 · 16 comments
Assignees
Labels
topic: GraphQL Related to Gatsby's GraphQL layer type: bug An issue or pull request relating to a bug in Gatsby

Comments

@johno
Copy link
Contributor

johno commented Aug 2, 2019

Description

Queries from themes that aren't being used cause an app to fail. It's sort of an abrupt experience to have installing a package make your app crash when it isn't configured. I'm not sure if we should check to make sure a theme is being used or if we should error in a more friendly way that says to configure the theme or use it.

Steps to reproduce

gatsby new my-themed-blog https://github.com/gatsbyjs/gatsby-starter-blog-theme
cd my-theme-blog
yarn add gatsby-theme-documentation
yarn start

Expected result

The blog theme runs unaffected by the docs theme which isn't configured.

Actual result

We receive an error that crashes the app:

 ERROR #85907  GRAPHQL

There was an error in your GraphQL query:

- Unknown field 'docs' on type 'Query'.

File: node_modules/gatsby-theme-documentation/src/templates/doc.js

image

Related

@johno johno added the type: bug An issue or pull request relating to a bug in Gatsby label Aug 2, 2019
@swyxio
Copy link
Contributor

swyxio commented Aug 2, 2019

would definitely like gatsby to do a check against gatsby-config before running stuff in node_modules for me.

@alexluong
Copy link
Contributor

alexluong commented Aug 2, 2019

I think it's the same for source plugins too. Like gatsby-source-contentful and gatsby-plugin-sharp. What @sw-yx seems like a good solution, but it may/may not run into issues with dependency of dependency (gatsby-theme-using-contentful depends on gatsby-source-contentful).

@swyxio
Copy link
Contributor

swyxio commented Aug 2, 2019

recursive check 🤷‍♂️

nothing is impossible. bottom line is are you surprising your users? yes? is it likely? yes? are you telling them what to do when it happens? no? probably not good.

@sidharthachatterjee
Copy link
Contributor

sidharthachatterjee commented Aug 3, 2019

This is unfortunate! We currently parse js in node_modules for all deps with gatsby in their name.

The reason we don’t work backwards from gatsby-config is for the ability to support queries in arbitrary dependencies (like gatsby-seo for example)

The right way to solve this would be to further filter for:

  • files in themes in specified in gatsby-config recursively or
  • files in the entire require tree (for deps like gatsby-seo)

@swyxio
Copy link
Contributor

swyxio commented Aug 3, 2019

a filter may not be the only approach available to you. i wonder if query parsing can be made lazier. right now queries are eager, which is ALSO a problem if you have a query in a file that is inside your main project but that isn't imported in anywhere, e.g. i would not expect my query inside unused_file.js to be run, but it does, and ofc can crash gatsby, which is no bueno.

making queries lazy would solve both this and that problem.

@alexluong
Copy link
Contributor

If I understand Gatsby right, the way it works is that it will run all the query at build time and generates the data into static json. That’s how in prod there is no graphql server.

Not sure how to parse the queries in a lazy way then.

@KyleAMathews
Copy link
Contributor

Yeah we don't run the queries, is the validation step. Perhaps we could store the errors for queries and only show them if we know the query is used in a component.

@progital
Copy link

progital commented Aug 4, 2019

I wanted to file a similar issue but @johno was faster. My issue is what @sw-yx described above. Is there any workaround or some hints where to start with fixing this validation issue?

@lannonbr lannonbr added the topic: GraphQL Related to Gatsby's GraphQL layer label Aug 14, 2019
@gatsbot
Copy link

gatsbot bot commented Sep 4, 2019

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.

If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!

As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@gatsbot gatsbot bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Sep 4, 2019
@johno johno added not stale and removed stale? Issue that may be closed soon due to the original author not responding any more. labels Sep 4, 2019
@swyxio
Copy link
Contributor

swyxio commented Sep 4, 2019

the "workaround" i use is uninstalling the node module and renaming js files. i literally delete ".js" so that gatsby stops doing its thing. its not good. too magic. i hope gatsby prioritize this as its a nasty experience for beginners who run into it (and it doesnt take long to do that).

@shadcn
Copy link
Contributor

shadcn commented Nov 26, 2019

Hitting this issue as well. @johno is there a way around this for latent shadowing? Currently shadowing a query component but this fails when the dependent theme is in node_modules but not configured.

@shadcn
Copy link
Contributor

shadcn commented Nov 26, 2019

OK I created a PR that checks for themes against gatsby-config as per @sidharthachatterjee suggestion above. Seems to be working for now.

Any pitfalls if unused themes are excluded from the query compiler?

@pvdz
Copy link
Contributor

pvdz commented Nov 28, 2019

Anyone know how common this case is? Do people often have unused themes in their build? Or mostly when initially setting up? Wondering about the impact surface of this problem.

@ukch
Copy link

ukch commented Nov 28, 2019

@KyleAMathews I am being hit by this sort of error from disabled components. I would think your proposed solution would resolve my problem (whereas I suspect the linked PR would not)

@rogermparent
Copy link
Contributor

@pvdz I'm running into an issue related to this which is less breaking but possibly more common in theme development. If a theme developer allows page queries to be replaced through shadowing or other means, the original query will still show the "GraphQL query in the non-page component" warning despite the theme being used as intended.
Ultimately, solving this variant of the issue is mostly to assuage the worries of developers consuming themes who may incorrectly think something is wrong, so while it expands the pool of cases it only does so with cosmetic issues.
I think @KyleAMathews proposal would fix this as well.

@LekoArts
Copy link
Contributor

Hi!

I'm closing this as a stale issue as in the meantime Gatsby 4 and related packages were released. You can check our Framework Version Support Page to see which versions currently receive active support.

Please try the mentioned issue on the latest version (using the next tag) and if you still see this problem, open a new bug report. It must include a minimal reproduction.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: GraphQL Related to Gatsby's GraphQL layer type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

Successfully merging a pull request may close this issue.