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

fix: specify gql schema to prevent cover inferrence error #36

Closed
wants to merge 1 commit into from

Conversation

russmatney
Copy link

Relevant: #21

I'm new to Gatsby, so am not confident this is the right way to solve
this. In particular, my reading led me to expect to only need to specify
the types down to the cover field, while the rest should be inferred.
However, doing this led to errors - the other types couldn't be found,
and a few other warnings showed up that might be relevant.

Doesn't feel like the ideal solution, but it solves the error I had, so
I thought I'd at least get this PR open.

Relevant links:

The root of the error is the cover field being detected as an empty
string (hence, String as the gql type) in some frontmatters before the
File type can be inferred from another post.

Gatsby opened APIs to allow for overriding types like these, but they're
supposed to merge your customized types with the inferred fields
somewhere in the pipeline - that does not appear to be happening here,
not sure why.

@ganevdev
Copy link
Member

Thanks for the PR!

Yes, this is an interesting idea, perhaps it will lead to a solution to the problem. We need to explicitly show that the cover is a file, not a string or anything else.

I can’t approve this PR, when I try to use it, the content of the site disappears. I tried various options, but either an error occurs or the content disappears.

@russmatney
Copy link
Author

Ah, I'm glad you checked - I can dig more into it later this week

@ganevdev
Copy link
Member

ganevdev commented Feb 19, 2020

Ah, I'm glad you checked - I can dig more into it later this week

That would be awesome. Even any intermediate results will be useful.

@russmatney
Copy link
Author

Sorry for the delay here - I spent a while looking into this today,
reading gatsby's docs and a handful of similar issues/prs in gatsby itself.

I've found quite a few people with these exact symptoms, and always the solution
presented is to use createSchemaCustomization to specify the type, and let
type inference do the rest. The below should be all that's necessary.

exports.createSchemaCustomization = ({ actions }) => {
  const { createTypes, createFieldExtension } = actions;
  createTypes(`
    type Mdx implements Node {
      frontmatter: MdxFrontmatter!
    }

    type MdxFrontmatter {
      cover: File @fileByRelativePath
    }
  `);
};

If you build this way, you'll see these warnings before many more missing field
errors.

warn Plugin `gatsby-plugin-mdx` tried to define the GraphQL type `MdxFrontmatter`, which has already been defined by the plugin `gatsby-theme-chronoblog`.
warn Plugin `gatsby-plugin-mdx` tried to define the GraphQL type `Mdx`, which has already been defined by the plugin `gatsby-theme-chronoblog`.

The problem is, type inference is not allowed in plugins, because by creating
these initial types, we are becoming the type's owners, and plugins are not
allowed to modify types that they do not own. I think this is not the desired
behavior and don't buy that graphQL type 'ownership' is a concept gatsby
should be concerned about, this issue being a great example of a time we want to
take advantage of GraphQL type inference rather than be 'protected' from it.
Maybe gatsby will fix this sometime, but it seems unlikely given the
conversations that have gone on already - a few PRs exist, but things are not
getting across the finish line. I don't understand why plugins are not allowed
to provide this feature, but, meh, not my codebase.

There seem to be workarounds that involve onCreateNode and
createNodeField to do this. Some plugins probably do it already, so it can
probably be done in gatsby-node.js as well. I tried briefly but did not want to
start guessing at how to update a node's field's fields. (Mdx>MdxFormatter>cover).
It feels like an unsupported hack, so might just lead to more surprises later,
but there's probably a way to do it.

I've updated my PR slightly, to get the type names correct - this gets the front
page to work, but viewing posts is still broken, likely because the hard-coded
type does not have all the features (resolvers, etc) it would get if the mdx plugin had created
it.

At the broadest perspective, this is a pretty minor issue, and hopefully it can
be handled with documentation or a better error message until a better fix can
be implemented. I do love the theme! Thanks much for putting it together!

Some relevant issues, if you want to dig any further:
gatsbyjs/gatsby#16928
gatsbyjs/gatsby#16529
gatsbyjs/gatsby#15544
gatsbyjs/gatsby#18271

@ganevdev
Copy link
Member

Thanks for the research @russmatney 👍

Now we at least know what we cannot do. Apparently, at the moment, there is no "elegant" or "right" solution.

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