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

Gatsby Schema Customization API fails with gatsby-source-contentful and union types #12832

Closed
sami616 opened this issue Mar 25, 2019 · 20 comments
Labels
type: bug An issue or pull request relating to a bug in Gatsby

Comments

@sami616
Copy link

sami616 commented Mar 25, 2019

Description

The new schema API always returns null on fields that reference multiple union types in Contentful when at least one of the types is not present.

I'm not sure if this is only affecting arrays of unions or single unions too.

in this instance the subSections field is not referencing any of the ContentfulSubSectionSibling union type

Steps to reproduce

clone: git clone https://github.com/sami616/gatsby-contentful-custom-schema-bug.git
yarn install
yarn start

Expected result

A list should render under the heading Section one

Actual result

My error message renders due to the subSections being null

Environment

System:
OS: macOS 10.14
CPU: x64 Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
Shell: 3.2.57 - /bin/bash
Binaries:
Node: 10.10.0 - ~/.nvm/versions/node/v10.10.0/bin/node
Yarn: 1.13.0 - ~/.nvm/versions/node/v10.10.0/bin/yarn
npm: 6.9.0 - ~/.nvm/versions/node/v10.10.0/bin/npm
Browsers:
Chrome: 73.0.3683.86
Firefox: 62.0.3
Safari: 12.0
npmPackages:
gatsby: ^2.2.10 => 2.2.10
gatsby-plugin-react-helmet: ^3.0.0 => 3.0.0
gatsby-source-contentful: ^2.0.42 => 2.0.42
gatsby-transformer-remark: ^2.1.7 => 2.1.7
npmGlobalPackages:
gatsby-cli: 2.4.3

@wardpeet
Copy link
Contributor

wardpeet commented Mar 25, 2019

cc @freiksenet @stefanprobst

#12272

@wardpeet wardpeet added the type: bug An issue or pull request relating to a bug in Gatsby label Mar 25, 2019
@sami616
Copy link
Author

sami616 commented Mar 28, 2019

Hi folks. No pressure but just wondering when this might be looked at? Want to manage expectations accordingly 👍🏻

@freiksenet
Copy link
Contributor

@sami616 Thank you for the report. Could you try if it works for you with this #12816? I'll take a look at the core issue tomorrow morning.

@sami616
Copy link
Author

sami616 commented Mar 28, 2019

@freiksenet nice! #12816 seems to handle missing references and fields great.

@freiksenet
Copy link
Contributor

@sami616 The core issue is that even with the created types, we currently rely on the inferred data to attach resolvers. When there is only one type, Gatsby can't yet know that this should be a union, so no resolver is added. I'm working on fixing it right now, plus in future sources with introspection (like contentful) will do this automatically. In the mean time, you could attach a manual resolver there.

exports.sourceNodes = ({ actions: { createTypes }, schema }) => {
  createTypes([
    `
    type ContentfulSubSectionSibling implements Node {
      title: String
    }

    type ContentfulSubSection implements Node {
      title: String
    }

    union ContentfulSubSectionContentfulSubSectionSiblingUnion = ContentfulSubSection | ContentfulSubSectionSibling


    type ContentfulPage implements Node {
      title: String
      slug: String
      sections: [ContentfulSection]
    }
  `,
    schema.buildObjectType({
      name: "ContentfulSection",
      interfaces: ["Node"],
      fields: {
        title: "String",
        subSections: {
          type: "[ContentfulSubSectionContentfulSubSectionSiblingUnion]",
          resolve(parent, args, context, info) {
            return context.nodeModel.getNodesByIds({
              ids: parent.subSections___NODE,
            });
          }
        }
      }
    })
  ]);
};

@sami616
Copy link
Author

sami616 commented Mar 29, 2019

Thanks @freiksenet - i just ran a small test and attached 2 out of 3 possible content types onto a field (instead of one like in my example) and it still failed - should gatsby at this point not know this should be a union?

ps - thank you for the example, attaching a manual resolver does seem to work

@freiksenet
Copy link
Contributor

freiksenet commented Mar 29, 2019

@sami616 the way it works is that it combines all the types present inside the ___NODE reference, meaning it will get a union of two types, not three types. It only updates resolvers if type names match and they won't between two and three union type.

This problem is one of the reasons we did the schema customization in the first place - inference works very badly for sources that already have introspection, because it depends so much on the data that was already added, not the structure user wants. When source-contenful PR lands this won't be a problem anymore, because contentful introspection will be the source of truth.

Before this lands explicit resolver is the recommended way to go and we are working on adding a shortcut for that.

@sami616
Copy link
Author

sami616 commented Mar 29, 2019

@freiksenet ah, okay i get it - awesome - well, very much looking forward to the fix! thanks again! 👍

@gatsbot
Copy link

gatsbot bot commented Apr 19, 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!

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 Apr 19, 2019
@sami616
Copy link
Author

sami616 commented Apr 24, 2019

reopen

@stefanprobst
Copy link
Contributor

with #13028 it should be possible to say:

type ContentfulSubSectionSibling implements Node {
  title: String
}

type ContentfulSubSection implements Node {
  title: String
}

union ContentfulSubSectionContentfulSubSectionSiblingUnion =
    ContentfulSubSection
  | ContentfulSubSectionSibling

type ContentfulSection implements Node {
  title: String
  subSections: [ContentfulSubSectionContentfulSubSectionSiblingUnion]
    @addResolver(type: "link", options: { from: "subSections___NODE" })
}

type ContentfulPage implements Node {
  title: String
  slug: String
  sections: [ContentfulSection]
    @addResolver(type: "link", options: { from: "sections___NODE" })
}

@sami616
Copy link
Author

sami616 commented Apr 25, 2019

HI @stefanprobst i tried installing gatsby@schema-customization and using your code above, but am now seeing Cannot read property 'split' of undefined in my terminal

@wardpeet
Copy link
Contributor

wardpeet commented Apr 25, 2019

could you update https://github.com/sami616/gatsby-contentful-custom-schema-bug so we can test it ourselves and create fix :)

@sami616
Copy link
Author

sami616 commented Apr 25, 2019

@wardpeet done! :)

@stefanprobst
Copy link
Contributor

@sami616 This should already work (needs a new alpha/beta release that contains #13591)

@gatsbot
Copy link

gatsbot bot commented May 6, 2019

Hey again!

It’s been 30 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.

Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to reopen this issue or create a new one if you need anything else.

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 again for being part of the Gatsby community!

@gatsbot gatsbot bot closed this as completed May 6, 2019
@pho3nixf1re
Copy link

There is still an in-progress PR that deals with this.

@stefanprobst stefanprobst reopened this May 6, 2019
@stefanprobst stefanprobst added not stale and removed stale? Issue that may be closed soon due to the original author not responding any more. labels May 6, 2019
@stefanprobst
Copy link
Contributor

With gatsby v2.5 the following should now work:

    type ContentfulSubSectionSibling implements Node {
      title: String
    }
    
    type ContentfulSubSection implements Node {
      title: String
    }
    
    union ContentfulSubSectionContentfulSubSectionSiblingUnion =
        ContentfulSubSection
      | ContentfulSubSectionSibling
    
    type ContentfulSection implements Node {
      title: String
      subSections: [ContentfulSubSectionContentfulSubSectionSiblingUnion]
        @link(from: "subSections___NODE")
    }
    
    type ContentfulPage implements Node {
      title: String
      slug: String
      sections: [ContentfulSection]
        @link(from: "sections___NODE")
    }

@stefanprobst
Copy link
Contributor

I'm closing this as it should be fixed - feel free to open if there are still issues. Thanks

@eddiecooro
Copy link
Contributor

Hey @stefanprobst
I have something similar to what you wrote, but now gatsby says:
Querying GraphQLUnion types is not supported.
Did I miss something?
Does it work if I write a custom resolver for it?

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

No branches or pull requests

7 participants