gatsbyjs / gatsby

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

[bug] ☂️ umbrella issue for schema customization issues #12272

Closed freiksenet closed 4 years ago

freiksenet commented 5 years ago

This is a meta issue for all the issues with 2.2.0, that were introduced by the schema refactoring.

What?

See the blog post for the details about why we did the refactoring and what's it all about.

See the release blog post for release notes and final updates.

How?

Install latest version of Gatsby and try running your site. Hopefully it will all just work. If you want, you could also try the two new APIs (createTypes and createResolvers).

yarn add gatsby

Changelog

gatsby@2.5.0

gatsby@2.5.0-rc.1

gatsby@2.4.0-alpha.2

gatsby@2.4.0-alpha.1

gatsby@2.2.0

gatsby@2.2.0-rc.2

gatsby@2.2.0-rc.1

gatsby@2.2.0-alpha.6

gatsby@2.2.0-alpha.5

gatsby@2.2.0-alpha.4

gatsby@2.2.0-alpha.3

exports.sourceNodes = ({ actions, schema }) => {
  const { createTypes } = actions
  createTypes([
    schema.buildObjectType({
      name: `CommentJson`,
      fields: {
        text: `String!`,
        blog: {
          type: `BlogJson`,
          resolve(parent, args, context) {
            return context.nodeModel.getNodeById({
              id: parent.author,
              type: `BlogJson`,
            })
          },
        },
        author: {
          type: `AuthorJson`,
          resolve(parent, args, context) {
            return context.nodeModel.getNodeById({
              id: parent.author,
              type: `AuthorJson`,
            })
          },
        },
      },
      interfaces: [`Node`],
    }),
  ])
}

gatsby@2.2.0-alpha.2

gatsby@2.2.0-alpha.1

gatsby@2.2.0-alpha.0

gatsby@2.1.20-alpha.0

rexxars commented 5 years ago

The filters for ID fields now expect an ID as input where they previously wanted a string. This breaks certain queries, for instance:

export const query = graphql`
  query BlogPostTemplateQuery($id: String!) {
    post: sanityPost(id: { eq: $id }) {
      id
      title
    }
  }
`

Will report:

error GraphQL Error Variable "$id" of type "String!" used in position expecting type "ID".

While it may be more correct to update the query to reflect this, it is a breaking change, so I thought I would report it.

freiksenet commented 5 years ago

@rexxars Nice find! I assume we used to convert ID filters to String. I'll restore old behaviour.

freiksenet commented 5 years ago

Released new version.

NicoleEtLui commented 5 years ago

Trying to query the schema in the resolver:

exports.createResolvers = ({ createResolvers, schema }) => {
  createResolvers({
    MenuJson: {
      someResolver: {
        type: `String!`,
        async resolve(source, args, context, info) {
          const foo = await graphql(schema, `
            {
              allPageJson {
                nodes {
                  id
                }
              }
            }
          `, {})

          console.log(foo)

          return 'WIP'
        },
      },
    },
  })
}
TypeError: Cannot read property 'nodeModel' of undefined
         at /private/tmp/test-gatsby/node_modules/gatsby/dist/schema/resolvers.js:22:15
         at /private/tmp/test-gatsby/node_modules/gatsby/dist/schema/resolvers.js:49:44
         at Generator.next (<anonymous>)
[...]
stefanprobst commented 5 years ago

@NicoleEtLui For queries in the field resolver, use the methods provided on context.nodeModel, i.e. you can use context.nodeModel.getAllNodes({ type: 'PageJson' }) or for more sophisticated queries you can use context.nodeModel.runQuery. There are some basic examples here.

If you need to access to the schema, note that the schema argument is just an intermediate representation - in the resolver you have access to the final built schema on info.schema.

freiksenet commented 5 years ago

Published gatsby@2.2.0-alpha.3

NicoleEtLui commented 5 years ago

@stefanprobst Thanks for the quick answer and all the great work you did !

LoicMahieu commented 5 years ago

Hi! Any suggestion to handle relationships between nodes ?

For example, files are stored in JSON (without id field, assume the file name as id): data/comments/some-uuid.json = { "message": "Hello", "postId": "some-post" } data/posts/some-post.json = { "content": "post" }

Using the source-filesystem and transformer-json plugins, that makes the nodes have an unpredictable ID since the transformer use createNodeId(). It makes difficult to find post from comment.

NicoleEtLui commented 5 years ago

Hi ! Trying to start gatsby project with any of these in gatsby-config.js:

will throw:

error Plugin gatsby-transformer-sharp returned an error

  Error: Cannot find module 'gatsby/dist/utils/cpu-core-count'

  - loader.js:581 Function.Module._resolveFilename
    internal/modules/cjs/loader.js:581:15

  - loader.js:507 Function.Module._load
    internal/modules/cjs/loader.js:507:25

  - loader.js:637 Module.require
    internal/modules/cjs/loader.js:637:17

  - v8-compile-cache.js:159 require
    [keemotion-corporate]/[v8-compile-cache]/v8-compile-cache.js:159:20
[...]
pieh commented 5 years ago

@NicoleEtLui Please update gatsby-plugin-manifest and gatsby-plugin-sharp, this was fixed in those packages with https://github.com/gatsbyjs/gatsby/pull/12332

freiksenet commented 5 years ago

Published gatsby@2.2.0-alpha.4

freiksenet commented 5 years ago

@LoicMahieu you can manually provide ids for the nodes, then you can do relationships by specifying fieldName___NODE fields.

freiksenet commented 5 years ago

Published gatsby@2.2.0-alpha.5

skinandbones commented 5 years ago

Q: Is this use case suitable for createResolvers?

I am using a remote CMS via gatsby-source-graphql which includes references to several remote files. I am currently pulling in those files with createRemoteFileNode. However, the page queries get awkward quickly because it's easy to get into situations where I need the result of the cms query (on the gatsby-source-graphql data source) to figure out which files I will need from gatsby-source-filesystem.

Ideally, I'd like to add/link/join (?) these remote file nodes into the cms nodes from gatsby-source-graphql. Is this a situation that createResolvers can help with?

stefanprobst commented 5 years ago

@skinandbones If I am understanding correctly, the short answer is "maybe but probably not quite yet".

We do support extending field configs on types added from a third-party schema, so it is possible to add a field resolver to a type from your CMS schema with createResolvers, and use createRemoteFileNode in the resolver. For example: https://github.com/stefanprobst/gatsby/blob/5bbfee29b5ec38f13a3070b13de4877aaddd6483/examples/using-gatsby-source-graphql/gatsby-node.js#L56-L71

The problem is that createRemoteFileNode will trigger onCreateNode API calls, and in the field resolver we have currently no way of knowing when those subsequent API calls have finished and it is safe for the resolver to return. (One approach to solve this could be #12202.) So depending on what exactly you intend to do with the remote files, this might or might not yet work.

stefanprobst commented 5 years ago

@LoicMahieu Do you have an example project you can link to?

skinandbones commented 5 years ago

@stefanprobst Yes, you understand correctly and the async issue makes sense. The example you linked is pretty similar to what I was expecting to do so I can give that a shot and see what happens. My plan is to run these file nodes through gatsby-transformer-sharp in the page query.

Is another viable approach to use createRemoteFileNode via the sourceNodes API (as usual) and then link those nodes into the 3rd party schema using the new API? Up to this point, I haven't been able to get into the 3rd party schema to do this.

stefanprobst commented 5 years ago

@skinandbones sorry, i should have clarified: the part in the example i linked to is currently not yet working, exactly because of the issue that when the field resolver returns, the File node will have been created, but the ImageSharp node (which is created in the triggered onCreateNode API call) not yet.

As for the second approach, I'd be interested in your findings -- it should be possible to query for the added remote File nodes in the resolver with context.nodeModel.getAllNodes({ type: 'File' }) or with something like context.nodeModel.runQuery({ type: 'File', query: { filter: { name: { regex: "/^remote/" } } } })

LoicMahieu commented 5 years ago

@stefanprobst Here is a example:

  1. https://github.com/LoicMahieu/test-gatsby-refactor-schema Here we get able to link comments to post by a complex lookup on parent File.

  2. https://github.com/LoicMahieu/test-gatsby-refactor-schema/tree/custom-transformer-json Here we get able to link them by using a custom JSON transformer where we could transform the object and also change the id. This method works but: if post is deleted and reference still exists in comments, gatsby will fail. It could be fixed by not use the ___NODE way but the new createResolvers: demo

baobabKoodaa commented 5 years ago

Hey, I just read this blog post about schema customization and the birthday example kinda seemed strange to me. In the example you're creating a custom resolver, which tries to resolve a date field into a Date, and falls back to a bogus value (01-01-1970) if the input date is not a proper Date. If you ever actually had a date in a real system, you would never want to replace incorrect inputs with bogus data. You would want to flag them as unknown/incorrect with something like null values. The fact that null wasn't used in the example made me wonder: is there some kind of limitation in Gatsby/GraphQL wrt. null values?

stefanprobst commented 5 years ago

@baobabKoodaa

is there some kind of limitation in Gatsby/GraphQL wrt. null values?

No. In GraphQL, you can explicitly set if a field should be nullable or not. More info here.

skinandbones commented 5 years ago

@stefanprobst I got this working and it works with ImageSharp. Very very cool and a game changer for working with a 3rd party schema 🎉 🎉

As for the second approach, I'd be interested in your findings -- it should be possible to query for the added remote File nodes in the resolver with context.nodeModel.getAllNodes({ type: 'File' }) or with something like context.nodeModel.runQuery({ type: 'File', query: { filter: { name: { regex: "/^remote/" } } } })

Here's what I did ...

exports.sourceNodes = async ({ actions, store, cache, createNodeId }) => {
  ... do createRemoteFileNode stuff ...
}

exports.createResolvers = ({ createResolvers, schema }) => {
  createResolvers({
    CMS_Thing: {
      thumbFile: {
        type: 'File!',
        async resolve(source, args, context, info) {
          const data = await context.nodeModel.runQuery({
            type: 'File',
            query: { filter: { fields: { ThingThumb: { eq: 'true' }, thingId: { eq: source.id } } } }
          })
          return data[0];
        }
      }
    }
  });
}

(This depends on me creating the file nodes with some fields, obviously. )

The optimal path (for my use case) will be to be able to use createRemoteFileNode in createResolvers so hopefully we can figure that out.

stefanprobst commented 5 years ago

@skinandbones Very cool! Btw, you can use firstOnly: true in runQuery to only get the first result.

freiksenet commented 5 years ago

Released gatsby@2.2.0-alpha.6.

freiksenet commented 5 years ago

Tentative goal is to merge this to master and release this next week. Please comment and try it :)

LoicMahieu commented 5 years ago

Seems that filters for fields of type resolved in a createResolvers() are not generated.

Example:

createResolvers({
  CommentsJson: {
    post: {
      type: `PostsJson`,
      resolve (source, args, context, info) {
        const allNodes = context.nodeModel.getNodeById({ id: source.postId, type: 'PostsJson' })
      }
    }
  }
})

CommentsJsonFilterInput type does not contains post field. So query like this could not work:

{
  allCommentsJson(filter: {post: {author: {eq: "foo"}}}) {
    nodes {
      id
    }
  }
}

Thanks

stefanprobst commented 5 years ago

@LoicMahieu This is intended behavior (at least for now): new fields added in createResolvers will not be present in the input filter, because createResolvers is run last in schema generation. The recommended approach here is to define a field type with createTypes action and then extend it in createResolvers - or use the recently added buildObjectType helper (see above).

Sorry that this is not yet better documented - for now you can have a look at the API docs directly in the branch: createResolvers and createTypes.

freiksenet commented 5 years ago

@LoicMahieu this is intentional, add resolvers happens after all processing and is meant as a tool to do final adjustments to the schema. You should add fields in createTypes if you want them to show in the filters. There is a new shorthand syntax that you can use, see the top post.

EDIT: Oops, didn't see @stefanprobst replying already :D

LoicMahieu commented 5 years ago

Thanks both for clarification. createResolvers and createTypes seems to achieve the same goal.


One thing that could be really useful is to save the generated schema. It would allow to "snapshot" the types and make sure that the schema will remain as we expect. I wrote a POC that seems to works well:

const { printType } = require("graphql")
const fs = require("fs-extra")
const path = require("path")

const schemaFilePath = path.join(__dirname, "./src/schema.gql")

exports.sourceNodes = async ({ actions }) => {
  const { createTypes } = actions

  if (await fs.exists(schemaFilePath)) {
    const typeDefs = (await fs.readFile(schemaFilePath)).toString()
    createTypes(typeDefs)
  }
}

exports.onPostBootstrap = async ({ store }) => {
  const { schema } = store.getState()
  const types = ["CommentsJson", "PostsJson"]
  const typeDefs = types
    .map(type => printType(schema.getType(type)))
    .join("\n\n")
  await fs.writeFile(schemaFilePath, typeDefs + "\n")
}

With this we can even delete all the data and the schema is still as we expect. Demo: https://github.com/LoicMahieu/test-gatsby-refactor-schema/commit/9696c8e386fd066262164f8e38f5689a14f111e1

stefanprobst commented 5 years ago

@LoicMahieu :+1: Something like this is part of our roadmap.

hilja commented 5 years ago

This does not touch the threading of schema building, right?

Referring to the comment: https://github.com/gatsbyjs/gatsby/issues/7373#issuecomment-413570634

Only one Node process working (this screencap is not from the beta release, just adding it here to illustrate my point):

Screenshot 2019-01-10 10 14 07
stefanprobst commented 5 years ago

@hilja The new schema customization API is not about running queries, but about schema generation, so this has not changed yet. There is experimental work going on to allow field resolvers to offload work to other processes, but that is not ready yet.

freiksenet commented 5 years ago

Released 2.2.0-rc.1. Soon.

m4rrc0 commented 5 years ago

Query sorting on a newly created fields seems buggy.

I can create a new field by doing:

exports.sourceNodes = ({ actions }) => {
  const { createTypes } = actions
  const typeDefs = `
    type MyNode implements Node {
      copyOfId: ID!
    }
  `
  createTypes(typeDefs)
}

exports.createResolvers = ({ createResolvers }) => {
  createResolvers({
    MyNode: {
      copyOfId: {
        resolve(source, args, context, info) {
          return info.originalResolver(
            {
              ...source,
              copyOfId: source.id,
            },
            args,
            context,
            info,
          )
        },
      },
    },
  })
}

But then if I query:

query {
    allMyNode (sort: {fields: [copyOfId]}) {
      nodes {
        copyOfId
    }
  }
}

results are not sorted and appear in original order.

If I sort on id everything is fine of course.

I guess this is the same problem as @LoicMahieu mentioned earlier but this is really misleading. If we can't do it then it has to be super well documented because it just seem natural to me to ease my filtering and sorting by creating the appropriate fields with these new API methods.

PS: forgot to mention... this new API is awesome!! :D Don't let my comment mislead you in thinking I am in a pessimistic mood. I'm just trying to be useful. ;) Thanks for the great work!

stefanprobst commented 5 years ago

@MarcCoet Thanks for testing! This is a known issue - we don't call field resolvers for sort fields currently, see #11368. It's the next thing we'll tackle once this is merged!

m4rrc0 commented 5 years ago

Oh, sorry. I didn't think about looking for setFieldsOnGraphQLNodeType issues. My bad. I am happy to know it is being worked on.

m4rrc0 commented 5 years ago

so there is really no way to get around this with the new API? We have to stick with the old ways of createNodeField?

I tried with buildObjectType but same result.

exports.sourceNodes = ({ actions, schema }) => {
  const { createTypes } = actions

  createTypes([
    schema.buildObjectType({
      name: `MyNode`,
      fields: {
        copyOfId: {
          type: `ID!`,
          resolve(parent) {
            return parent.id
          },
        },
      },
      interfaces: [`Node`],
    }),
  ])
}

Any other undocumented way to achieve sorting on new node fields by chance?

m4rrc0 commented 5 years ago

Appart of that I just had a syntax error with the @infer() directive. But I guess that is what 'Allow inference options on non-SDL types' means in the known issues above?!

stefanprobst commented 5 years ago

@MarcCoet

syntax error with the @infer() directive

Does this work for you?

createTypes('type MyNode implements Node @infer { foo: Boolean }')

As for the sorting issue: this problem is a bit orthogonal to the schema customization refactor. The issue is: when we want to filter or sort on fields that are not on the node object itself but added by field resolvers, we need to call those resolvers before so we have the full nodes available for filtering and sorting. E.g. in your example, the node object does not have a copyOfId field -- for it to be available we need to call the resolver before filtering and sorting. Currently, we only do this for filter fields, but not for sort fields. As a workaround you could try adding a dummy filter for copyOfId to your query:

query {
  allMyNode (sort: { fields: [copyOfId] }, filter: { copyOfId: { ne: null } }) {
    nodes {
      copyOfId
    }
  }
}

See also this comment.

kennedyrose commented 5 years ago

I'm trying to set the schema for the frontmatter nodes generated by gatsby-transformer-remark and always run into this error: Error: Schema must contain unique named types but contains multiple types named "MarkdownRemarkFrontmatter".

My schema looks like this:

type MarkdownRemarkFrontmatter implements Node {
  title: String
}
stefanprobst commented 5 years ago

@kennedyrose Thanks for testing! Does the following work for you?

exports.sourceNodes = ({ actions }) => {
  const { createTypes } = actions
  createTypes(`
    type MarkdownRemarkFrontmatter {
      title: String
    }
    type MarkdownRemark implements Node {
      frontmatter: MarkdownRemarkFrontmatter
    }
  `)
}

Only top-level types, i.e. node types generated by source and transformer plugins (like MarkdownRemark or ImageSharp) should implement the Node interface, not nested types like the frontmatter type. (Although we do seem to have a bug when targeting nested infered types directly)

kennedyrose commented 5 years ago

That works. Thanks!

freiksenet commented 5 years ago

Published gatsby@2.2.0-rc.2.

freiksenet commented 5 years ago

Published gatsby@2.2.0. Thank you everyone!

m4rrc0 commented 5 years ago

@stefanprobst

Does this work for you? createTypes('type MyNode implements Node @infer { foo: Boolean }')

It does! It is just the original article that is misleading then.

I guess the fog will raise with the arrival of proper documentation. ;)

About the sorting issue, thanks a lot for the clarifications. That is really helpful. The workaround seems to be working so this is perfect for now.

I am very excited about the new possibilities of Gatsby. Thanks a lot for the work you guys put in Gatsby.

DSchau commented 5 years ago

Want to x-post gatsbyjs/gatsby#12696, specifically this post

Looks like file inference/detection may have been slightly tweaked here? Updating to 2.2.2 broke, whereas ~2.1.0 seems to correctly infer the file node.

Wolfsun commented 5 years ago

Just found this when experimenting with creating custom interfaces using the API. Using the resolveType function on the interface works, but using isTypeOf functions instead on the objects that share the interface does not. I think this is down to the below code in schema.js, which assumes the resolveType should be node, when no function is declared. This assumption is now incorrect with custom interface types.

if (!typeComposer.getResolveType()) {
      typeComposer.setResolveType(node => node.internal.type);
    }

Great work by the way, just in time for my project!

stefanprobst commented 5 years ago

Hi @Wolfsun Yes, since providing resolveType via SDL isn't possible we provide a default which should work in most cases, and allow providing a custom one via graphql-js types. It's true this does not play well with isTypeOf. Out of curiosity: is there a reason for using isTypeOf over resolveType?

Wolfsun commented 5 years ago

Hi @stefanprobst, thanks for the explanation. Using resolveType is perfectly workable for me at this stage, however using with isTypeOf would be slightly cleaner as I have one interface with lots of implementing objects. I'm using a data structure to represent fairly complex grid based layouts, I have a Card interface, and many CardTypes implementing this interface, which are stored in the data structure relative to their position on the grid. As I don't know the CardType in advance an interface is required. Potentially isTypeOf could be useful if I want to use a plugin system for additional Cards, but I've not given this much thought yet, and it may not even be a good idea, so very much an issue for another day!

stefanprobst commented 5 years ago

For discoverability I'm linking to this example that maybe others find useful as well. Hope this is ok.