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

feat(gatsby-source-contentful): Add option to use Content Type ID for GraphQL schema generation #20265

Merged
merged 2 commits into from
Jan 6, 2020

Conversation

peabnuts123
Copy link
Contributor

Description

The gatsby-source-contentful plugin generates a GraphQL interface to Contentful. Fields are generated in the GraphQL schema based on the Content Types that are available in your Contentful space. At-present, the names of these fields are generated from the names of the content types in Contentful. As discussed in #9568 (and elsewhere, I'm sure) this is pretty fragile, as the name in Contentful is:

  1. User-facing i.e. impacts UX in Contentful
  2. Not necessarily controlled by the dev team, or by somebody who will inform the dev team of changes
  3. Likely to change every so often, especially early on in a project

Any changes to the display name in Contentful means refactoring to your project's code base in the following knock-on ways:

  1. Renaming GraphQL queries
  2. Renaming all references to the data from GraphQL queries
  3. (etc.) Renaming any references to generated TypeScript definitions

It's problematic that the display name is used for both natural-language UX as well as a unique identifier.

I understand this choice was originally made to combat the fact that Contentful will automatically generate you an ID if one is not specified at creation time (e.g. by not supplying one in the UI of the website, or when using the API) e.g. c6XwpTaSiiI2Ak2Ww0oi6qa. Using this ID for GraphQL and subsequent code references would obviously not be ideal.

This PR introduces a config flag to specify whether the developer wants to use the Name or the ID of their content types for the GraphQL schema. Obviously, backwards compatibility is a must, as we cannot force everybody upgrading this package to refactor their entire codebase, so the flag defaults to using the Name - keeping identical behaviour as before (this is evident in the test snapshots that none of them had to be updated).

The flag is called useNameForId and it defaults to true. The documentation suggests that people should disable this flag if they are using natural-language IDs, or leave it enabled if you have a lot of generated IDs.

The flag is structured this-way-around such that it may eventually default to false (a more natural default value). It seems to me that id is a much better property to be used as an identifier than name. Having an optional flag will still support people who need to deal with a lot of generated IDs. However, I don't know how we would migrate from true to false as the default; perhaps when the package leaps its next major semver.

Documentation

The README.md of the gatsby-source-contentful package has been updated to outline the config flag, what it does, and when developers should use true vs. false.

Related Issues

@peabnuts123 peabnuts123 requested review from a team as code owners December 22, 2019 19:56
Copy link
Contributor

@vladar vladar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems reasonable, and the code looks good to me. But I don't have enough context on Contentful plugin to merge this, so let's wait for another pair of eyes.

@pvdz pvdz changed the title Allow gatsby-source-contentful to use Content Type ID for GraphQL schema generation feat(gatsby-source-contentful): Add option to use Content Type ID for GraphQL schema generation Jan 6, 2020
@pvdz
Copy link
Contributor

pvdz commented Jan 6, 2020

Agreed. And it should be fine since it's opt-in.

@pvdz pvdz merged commit 0e71cfb into gatsbyjs:master Jan 6, 2020
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.

Is there a reason why gatsby-source-contentful uses content type Names in graphql instead of IDs?
3 participants