gatsbyjs / gatsby

The best React-based framework with performance, scalability and security built in.
https://www.gatsbyjs.com
MIT License
55.27k stars 10.32k forks source link

gatsby-source-contentful: if media is video or image, non-nullable field breaks build #20696

Closed danoszz closed 4 years ago

danoszz commented 4 years ago

Description

For the past months, I have been using Contentful and Gatsby successfully. Somehow, a couple of days ago, my build keeps breaking with the following error.

ERROR #85925  GRAPHQL

There was an error in your GraphQL query:

Cannot return null for non-nullable field ContentfulFluid.aspectRatio.

The field "ContentfulFluid.aspectRatio." was explicitly defined as non-nullable via the schema customization API (by yourself or a plugin/theme). This means that this field is not optional and you have to define a value. If this is not your desired behavior and you defined the schema yourself, go to "createTypes" in gatsby-node.js. If you're using a plugin/theme, you can learn more here on how to fix field types:
https://www.gatsbyjs.org/docs/schema-customization/#fixing-field-types

   1 | fragment GatsbyContentfulFluid_noBase64 on ContentfulFluid {
>  2 |   aspectRatio
     |   ^
   3 |   src
   4 |   srcSet
   5 |   sizes
   6 | }
   7 |
   8 | query ProjectPostBySlug($slug: String!) {
   9 |   site {
  10 |     siteMetadata {
  11 |       title
  12 |     }

File path: /Users/daan/Development/Devign/daan-van-der-zwaag/src/templates/ProjectPost/index.js
Url path: /projects/accept-uncertainty/
Plugin: none

Context

I've set up a portfolio website, where each project has its own page. On this project page, I display videos and images about the project. To have a smooth workflow in Contentful, I've defined one content type showcaseMedia for all assets. Here you can upload media with some metadata and options. This uploaded asset can be both a video and an image.

Screen Shot 2020-01-18 at 16 28 47

I call the following GraphQL query on the project page to access the data. The below query is simplified, the full query can be found here.

export const ProjectPostQuery = graphql`
    query ProjectPostBySlug($slug: String!) {
        site {
            siteMetadata {
                title
            }
        }
        contentfulProjectPosts(slug: { eq: $slug }) {
            showcaseMedia {
                media {
                    file {
                        contentType
                        url
                    }
                    fluid(maxWidth: 1440) {
                        ...GatsbyContentfulFluid_noBase64
                    }
                }
                isVideo
                videoURL
                needFrameDesktop
                title
                extensiveDescription {
                    extensiveDescription
                }
            }
        }
    }
`;

Later I differentiate if the media item is an image or video with the boolean isVideo. For an image, I will use the Img component from gatsby-image, for displaying a video I use a video or iframe element.

const MediaItems = ({ source }) => {
    return (
        <div>
            {source.map(({ node }, i) => (
                <div className={source[i].isVideo ? "body--showcase-item video" : "body--showcase-item"} key={i}>
                    {source[i].needFrameDesktop && (
                        <div className="frame--top">
                            <div className="dot" />
                            <div className="dot" />
                            <div className="dot" />
                        </div>
                    )}
                    {source[i].isVideo ? (
                        <div className={source[i].needFrameDesktop ? "video--wrapper hasFrame" : "video--wrapper"}>
                            {source[i].videoURL ? (
                                <iframe
                                    src={source[i].videoURL}
                                    frameBorder="0"
                                    allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture"
                                    allowFullscreen
                                    title={source[i].title}
                                />
                            ) : (
                                <video playsInline loop controls autoPlay muted>
                                    <source src={source[i].media.file.url} type="video/mp4" />
                                    <p>
                                        Video could not be found. <a href="mailto:daan@devign.it">Please let me know</a>
                                    </p>
                                </video>
                            )}
                        </div>
                    ) : (
                        <div className={source[i].needFrameDesktop ? "image--wrapper hasFrame" : "image--wrapper"}>
                            <Img
                                key={i}
                                className="showcase-item-image"
                                alt={source[i].title}
                                fluid={source[i].media.fluid}
                            />
                        </div>
                    )}
                    {source[i].title && source[i].extensiveDescription ? (
                        <div className="showcase-item--description">
                            <h4>{source[i].title}</h4>
                            <p className="project-item--description small--text">
                                {source[i].extensiveDescription.extensiveDescription}
                            </p>
                        </div>
                    ) : null}
                </div>
            ))}
        </div>
    );
};

🤔While I am sure this code is not perfect and efficient, it worked like a charm for the past months. Both in development and production. Only now, it is breaking the build locally and on Netlify.

Steps to reproduce

  1. Setup a new content type in Contentful named showcaseMedia (or pick another name)
  2. Add a new entry for showcaseMedia an upload an image
  3. Create a second entry for showcaseMedia an upload a video here.
  4. Display both showcaseMedia items on one parent page, such a project post page
  5. In your Gatsby project, query your new data.
query YourQuery {
  contentfulProjectPosts {
    showcaseMedia {
      media {
        file {
          url
        }
        fluid(maxWidth: 1440) {
         ...GatsbyContentfulFluid_noBase64
        }
      }

    }
  }
}
  1. You will get an error message saying Cannot return null for non-nullable field ContentfulFluid.aspectRatio. and your code chunk where the error originates.

Expected result

  1. Displaying both images and videos should be possible within the same content type.
  2. The build should not be breaking by when the data is null.
  3. Handling nullable fields should be done on the Frontend.

Actual result

Problem origins When removing new GraphQLNonNull() at fluidNodeType located gatsby/packages/gatsby-source-contentful/src/extend-node-type.js, everything works as excepted.

The following threads talk about the related/same problem.

https://github.com/gatsbyjs/gatsby/issues/1517 https://github.com/gatsbyjs/gatsby/issues/2881 https://github.com/gatsbyjs/gatsby/issues/3344

Possible solutions I can imagine some fields need to be NonNullable. So by just removing them, it will cause more problems in the future. That's why I thought about some solutions, but I am not able to get my head around what the correct solution will be.

  1. Conditionally loading certain fields from GraphQL
  2. Allowing null value for the field type fluidNode by setting an option in gatsby-config.js
  3. Possibility to redefine the schema for the ContentfulFluid fragment in gatsby-node.js

I don't mind to make a PR to fix this matter, but I'd like to have some advice on

  1. How to fix this issue, so I can continue deploying the website on Netlify w/o breaking?
  2. If making a PR, which solution can work the best?

Environment

gatsby info --clipboard

  System:
    OS: macOS 10.15.2
    CPU: (4) x64 Intel(R) Core(TM) i5-7287U CPU @ 3.30GHz
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 12.12.0 - /usr/local/bin/node
    Yarn: 1.19.1 - /usr/local/bin/yarn
    npm: 6.13.6 - /usr/local/bin/npm
  Languages:
    Python: 2.7.16 - /usr/bin/python
  Browsers:
    Chrome: 79.0.3945.117
    Firefox: 66.0.3
    Safari: 13.0.4
  npmPackages:
    gatsby: ^2.18.25 => 2.18.25
    gatsby-cli: ^2.8.27 => 2.8.27
    gatsby-image: ^2.2.39 => 2.2.39
    gatsby-plugin-dark-mode: ^1.1.0 => 1.1.0
    gatsby-plugin-google-analytics: ^2.1.33 => 2.1.33
    gatsby-plugin-manifest: ^2.2.37 => 2.2.37
    gatsby-plugin-offline: ^3.0.32 => 3.0.32
    gatsby-plugin-react-helmet: ^3.1.21 => 3.1.21
    gatsby-plugin-sass: ^2.1.27 => 2.1.27
    gatsby-plugin-sharp: ^2.3.13 => 2.3.13
    gatsby-plugin-transition-link: ^1.17.7 => 1.17.7
    gatsby-plugin-typescript: ^2.1.26 => 2.1.26
    gatsby-plugin-use-dark-mode: ^1.1.2 => 1.1.2
    gatsby-source-contentful: 2.1.77 => 2.1.77
    gatsby-source-filesystem: ^2.1.46 => 2.1.46
    gatsby-transformer-remark: ^2.6.48 => 2.6.48
    gatsby-transformer-sharp: ^2.3.13 => 2.3.13
  npmGlobalPackages:
    gatsby-cli: 2.8.27
daydream05 commented 4 years ago

Having this same issue! Such a really frustrating bug. I suggest people shouldn't clear their cache until this is resolved.

vladar commented 4 years ago

So the expectation is that the field fluid must return null when media is not an image:

{
  media: {
    fluid: null
  }
}

But instead, it returns an object where all fields are null:

{
  media: {
    fluid: {
      aspectRatio: null,
      // all other fields are null as well
    }
  }
}

It happens in fluid resolver (and in fixed too):

https://github.com/gatsbyjs/gatsby/blob/2a1508e3175082ec559a5405ba464bf3c9156128/packages/gatsby-source-contentful/src/extend-node-type.js#L495-L503

This was working in the past because all fields of ContentfulFluid and ContentfulFixed were nullable. This was changed in #20314

The good fix would be to check if the node is null in the resolver above and return null vs object. Does anybody want to do a PR for this? Ideally, we should also have a test case for this.

ryanhefner commented 4 years ago

@vladar This should be resolved via #20794 .

@danoszz, @daydream05 This fix has been published. Please let me know if it resolves your issues.

danoszz commented 4 years ago

@ryanhefner yessir! Thanks for the quick fix 🏄‍♂️