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

[source-contentful] handling undefined values #1517

Closed
m4rrc0 opened this issue Jul 15, 2017 · 40 comments
Closed

[source-contentful] handling undefined values #1517

m4rrc0 opened this issue Jul 15, 2017 · 40 comments

Comments

@m4rrc0
Copy link
Contributor

m4rrc0 commented Jul 15, 2017

Would be nice to have a way to query an entry field regardless of it being defined in contentful.
I mean if a field exists in Contentful but is not required (and empty) I would expect getting undefined or null but now the request to Contentful seems to fail.

TypeError: Cannot read property 'contentfulSettings' of undefined
    at graphql.then.result (/home/marc/code/TOILE/toile.io/gatsby-node.js:120:24)
    at tryCatcher (/home/marc/code/TOILE/toile.io/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/home/marc/code/TOILE/toile.io/node_modules/bluebird/js/release/promise.js:512:31)
    at Promise._settlePromise (/home/marc/code/TOILE/toile.io/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromiseCtx (/home/marc/code/TOILE/toile.io/node_modules/bluebird/js/release/promise.js:606:10)
    at Async._drainQueue (/home/marc/code/TOILE/toile.io/node_modules/bluebird/js/release/async.js:138:12)
    at Async._drainQueues (/home/marc/code/TOILE/toile.io/node_modules/bluebird/js/release/async.js:143:10)
    at Immediate.Async.drainQueues (/home/marc/code/TOILE/toile.io/node_modules/bluebird/js/release/async.js:17:14)
    at runCallback (timers.js:800:20)
    at tryOnImmediate (timers.js:762:5)
    at processImmediate [as _immediateCallback] (timers.js:733:5)
@chrisk2020
Copy link

I noticed this is also the case using [source-wordpress]

@KyleAMathews
Copy link
Contributor

From the stack trace it looks like the error is in your gatsby-node.js file. So the query is succeeding. Correct?

@m4rrc0
Copy link
Contributor Author

m4rrc0 commented Jul 20, 2017

I have this query in my gatsby-node.js

exports.createPages = ({ graphql, boundActionCreators }) => {
  const { createPage } = boundActionCreators
  return new Promise((resolve, reject) => {
    resolve(
      graphql(
        `
        {
          contentfulSettings {
            url
            name
            < ... more here ... >
          }
        }
        `
      ).then(result => {
        if (result.errors) {
          reject(result.errors)
        }

        const {
          url,
          name,
          < ... more here ... >
        } = result.data.contentfulSettings

...

If I have both the url and name fields defined in contentful, everything works fine. If one of the fields is empty however, then result.data is undefined.

@sebastienfi
Copy link
Contributor

That's true, the query will fail if the field does not exists in the node's GraphQL schema.

Do you expect it to work like a reference to an undefined Javascript's Object property would ? It yes, then I don't think that's Gatsby related, the answer lays into the deep understanding of GraphQL's Core.

@m4rrc0
Copy link
Contributor Author

m4rrc0 commented Jul 23, 2017

Good point @sebastienfi . Your comment made me dig a little.
I did not dug into the plugin code (graphQL is not my bff yet) but from my observations I am guessing we infer node's GraphQL schema from the Contentful entries directly, right?
As Contentful does not return empty fields in its response it is then absent from the schema.
What if we inferred the schema from the Contentful content model instead so that we are sure that every field present in the content model is callable in Gatsby and if a specific field value is absent from the response we define it as null or undefined or whatever goes well with graphQL.
What am I missing?

@sebastienfi
Copy link
Contributor

Contentful does not return empty fields in its response

It seems to me that this is the answer. It is not in the response because Contentful seems to ommit fields which has either null or empty value, so Gatsby and GraphQL both doesn't know it exists.

@m4rrc0
Copy link
Contributor Author

m4rrc0 commented Jul 24, 2017

My point exactly. So what do you think about inferring the graphQL schema from the content model?

My argument is that Contentful is a CMS. It is meant to be used by editors. If a field is not required in contentful the editor will reasonably understand that the field can be empty. But it cannot as the graphQL query will fail in that case! It seems like a real problem to me.

Is there maybe a way to tell Contentful to provide explicit responses for each field, even undefined ones @Khaledgarbaya ?

@KyleAMathews is it the same issue you were mentionning here #1264?

@dan-weaver
Copy link
Contributor

dan-weaver commented Jul 28, 2017

This just bit me as well. Is #1264 addressing this? Not sure you guys are understanding what @MarcCoet is saying? forgive me if I'm wrong.

But, it seems that if a field is specified in a query that doesn't happen to exist in any of the contenful records returned at that time by the api, then the graphql schema is not able to service that field, because it doesn't know it exists?

So, I'm guessing the schema is currently being build from existing records. Is that not correct? Could the schema be built instead from the content type descriptions themselves: https://www.contentful.com/developers/docs/references/content-delivery-api/#/reference/content-types/content-model . I was having trouble deciphering exactly what was going on. Would love to help out with this!

@KyleAMathews
Copy link
Contributor

Yeah Gatsby builds the schema from the data it has. The plugin does use the content schema to identify markdown fields. Would be happy to take a PR that also uses the content model to identify fields that don't have data yet.

@dan-weaver
Copy link
Contributor

@MarcCoet i'm trying to take a stab at this now, let me know if you've already made progress though.

@m4rrc0
Copy link
Contributor Author

m4rrc0 commented Aug 3, 2017

Awesome @dan-weaver . No unfortunately August is just a long rush for me. No time to investigate. =/
I'd be glad to test anything you have and report back though.

@m4rrc0
Copy link
Contributor Author

m4rrc0 commented Aug 3, 2017

And BTW, if you have any thought about #1703 . I'd love to read it.

@Khaledgarbaya
Copy link
Contributor

👋 hey there, Sorry I was off for two weeks, I totally agree on having the schema defined based on the content type and it should be fairly easy to that.
we have a labs project that allows you to query the data stored in Contentful using graphql, A schema and value resolvers are automatically generated out of an existing space. Please check the source code because there is a lot of useful stuff there.
I will be glad to answer any question related to Contentful, but unfortunately I don't have time to make a PR for that

@dan-weaver
Copy link
Contributor

@Khaledgarbaya thanks. I've been procrastinating.. Will let you know if I have any questions! thanks for the examples

@m4rrc0
Copy link
Contributor Author

m4rrc0 commented Sep 1, 2017

Hey @dan-weaver , any news from your side?
I'll try and get my head around this next week. I've got a few thinks to learn before I can be productive with graphql but I'll definitely have something at some point and it will need a review. ;)
I'll let you know when I get there.

@iw-dweaver
Copy link

@MarcCoet hey sorry for the lack of update. I tried for a bit but probably need some guidance. Also was having trouble with gatsby's development workflow.. Mosty just nuisances though in that regard.

My biggest question at this point is where should this be done

Options as far as I can tell:

  1. within setFieldsOnNodeTypes
  2. setting these fields during the normal createNode routines in normalize.js

I think I'm lacking some understanding of how gatsby constructs its GraphQL types.

@m4rrc0
Copy link
Contributor Author

m4rrc0 commented Sep 1, 2017

Maybe @KyleAMathews could give you some clues?
Never coded a plugin (yet) so I need a bunch of hours on the problem before I can understand your question. ;)

@dan-weaver
Copy link
Contributor

Yeah that'd be great. I know this probably isn't huge priority, but it would be a good learning experience.. I have some other plugins I'd like to write in the future. Hopefully my question above isn't too vague.

@KyleAMathews
Copy link
Contributor

So probably the best way to "fill in" missing fields in the schema would be @iw-dweaver's (1) option to use setFieldsOnGraphQLNodeType. You'd look at each generated schema and compare the schema against the Contentful model. If there's any missing fields, you add that field with the correct type.

@3CordGuy
Copy link
Contributor

3CordGuy commented Sep 5, 2017

Trying to use contentful as a front-end for my wife's blog. I also can't rely on her to always use every field defined in the content schema. Would be great if there was an intuitive solution for this.

@jquense
Copy link
Contributor

jquense commented Sep 5, 2017

The best way to handle these cases for possibly undefined fields is, as Kyle mentioned, to use the setFieldsOnGraphQLNodeType api to explicitly define the fields so they exist whether or not they are empty

@dan-weaver
Copy link
Contributor

Thanks folks. Just FYI @MarcCoet I'm going to set aside some time to dive into this again today / tomorrow, but don't let me be your blocker if it's urgent. And let me know if you've already got something.

@Khaledgarbaya
Copy link
Contributor

If I am not wrong you can here check the fields of the entry and the fields in the contentType, if one the of the fields does not exist you set it to null or something similar.

@dan-weaver
Copy link
Contributor

@Khaledgarbaya I had been going down that route, but to no avail. See my "option 2 above". Someone please correct me if I'm wrong, because I'd love to know. But from looking at the code, it seems Gatsby would not have enough information about the field type if it's simply given a null value there. As far as I could tell the fields were just being dropped when i did that.

@m4rrc0
Copy link
Contributor Author

m4rrc0 commented Sep 6, 2017

@dan-weaver just to let you know asap that I think I've got a working solution... I hope I didn't miss something huge because I made it quick and dirty.
Basically I went Khaled's way and set a value directly when entryNodes are built.
I will post a PR as soon as possible.

@Khaledgarbaya
Copy link
Contributor

@MarcCoet That's Great news, feel free to mention me in the PR and I'll try to help there

@HZSamir
Copy link

HZSamir commented Oct 11, 2017

@MarcCoet Still no progress about his issue?

@m4rrc0
Copy link
Contributor Author

m4rrc0 commented Oct 21, 2017

Hey @Unforgiven-wanda the PR has been left hanging because we need to implement some tests and I don't know anything about tests.
My suggestion was to add problematic cases to the test space but got no answer back.
I am running the PR code in a local plugin and it is all good for me (at least for what it was supposed to solve).
No progress on images and unused content types (e.g. blogPost content type exists but no blog post is created) though.
I think #2518 is mentioning the same kind of issue for images

@stefanoverna
Copy link
Contributor

This single issue is preventing our agency to extensively use Gatsby on every new project.. I would be happy to help proposing a PR but I'm not sure how to proceed :(

@KyleAMathews
Copy link
Contributor

@stefanoverna would love to see you using Gatsby for every project! @pieh is working on a fix in #3344 — perhaps you could chat with him to see if you could help out? He's got a branch he's working on that you could look at.

@SeanRoberts
Copy link

SeanRoberts commented Mar 15, 2018

Is @pieh's schema solution being looked at as the path forward vs. altering gatsby-source-contentful to generate schemas from the Contentful's content model?

@MarcCoet is there any way to use your untested plugin in the meantime? This is a showstopper for us, so would love to have a solution even if it's a little shaky 🙂

@pieh
Copy link
Contributor

pieh commented Mar 16, 2018

@SeanRoberts complete solution would allow gatsby-source-contentful to use content model to generate schema - my "solution" was just proof of concept that it can feasibly be done and actually work. I decided to just use schema definition language there but it could be expanded to add api for plugins to define schema types programatically.

@KyleAMathews
Copy link
Contributor

Due to the high volume of issues, we're closing out older ones without recent activity. Please open a new issue if you need help!

@ErisDS
Copy link
Contributor

ErisDS commented Sep 10, 2018

For anyone else stumbling across this it's still open/tracked here: #3344

@Khaledgarbaya
Copy link
Contributor

Hey Folks,
I haven't tested this but this might be a p[otential Clean workaround that does not require creating dummy content.

https://medium.com/@Zepro/contentful-reference-fields-with-gatsby-js-graphql-9f14ed90bdf9

@sami616
Copy link

sami616 commented Jan 17, 2019

@Khaledgarbaya this works great for optional content types but we quickly run into the issue again if one of those types have optional fields that are undefined

@gusliedke
Copy link

This is still an issue

@crosumo
Copy link

crosumo commented Nov 14, 2019

For me its an issue as well. If i have 300 posts and i dont choose a category (or other kind of field) at least in 1, i will have the issue.

@m4rrc0
Copy link
Contributor Author

m4rrc0 commented Nov 15, 2019

You should try the gatsby-plugin-schema-snapshot. I personally still have issues with Contentful but we are definitely heading in the right direction.

@sonusynup
Copy link

I'm still having issue with graphql schema when I run query against non-existing data(Editor might have unpublish the data from Contentful). Any workaround for it????

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

No branches or pull requests