fix: specify gql schema to prevent cover inferrence error

russmatney commented 4 years ago

Relevant: https://github.com/Chronoblog/gatsby-theme-chronoblog/issues/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 commented 4 years ago

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 commented 4 years ago

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

ganevdev commented 4 years ago

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 commented 4 years ago

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: https://github.com/gatsbyjs/gatsby/pull/16928 https://github.com/gatsbyjs/gatsby/issues/16529 https://github.com/gatsbyjs/gatsby/issues/15544 https://github.com/gatsbyjs/gatsby/issues/18271

ganevdev commented 4 years ago

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.

Chronoblog / gatsby-theme-chronoblog

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