datocms / gatsby-source-datocms

Official GatsbyJS source plugin to pull content from DatoCMS
MIT License
140 stars 50 forks source link

createNodeField does not create a field #92

Closed stzonis closed 4 years ago

stzonis commented 4 years ago

Hi, I am trying to add a field to an existing DatoCMS node but it doesn't work.

I have added this code in the file gatsby-node.js:

exports.onCreateNode = ({ node, actions }) => {
  const { createNodeField } = actions
  if (node.internal.type === "DatoCmsPerson") {
    // console.log(node) // the node extists
    createNodeField({
      node,
      name: "hello",
      value: "hello!!!",
    })
  }
}

Any ideas why I don't see the field in GraphiQL?

matjack1 commented 4 years ago

hey @stzonis

The only thing that I've found that you can try is to change onCreateNode to this:

exports.onCreateNode = ({ node, getNode, actions }) => {
  const { createNodeField } = actions
}

?

stzonis commented 4 years ago

hey @matjack1

It still doesn't work with your change...

Many thanks for your help!

matjack1 commented 4 years ago

I'm going to close this as I think it's more related with Gatsby rather than with our sourc plugin, but if you instead think it has to do with Dato feel free to comment again and provide more info on this. Sorry I cannot help you more here.

stzonis commented 4 years ago

Ok!

dbismut commented 4 years ago

@matjack1 I believe this is definitely linked to this plugin, not Gatsby.

v2.1 seems to break onCreateNode or createNodeField. It worked fine with 2.0.0. This is I believe critical.

stefanoverna commented 4 years ago

Reopening!

winstonma commented 4 years ago

Yeah I got this problem too. I tried to download the image from remote and served locally. Here is my code.

exports.onCreateNode = async ({
  node,
  actions,
  store,
  cache,
  createNodeId
}) => {
  if (node.internal.type === `DatoCmsVideoItem`) {
    const { createNode, creNodeField } = actions;
    const imageURL = node.videoId;

    let fileNode = await createRemoteFileNode({
      url: imageURL, // string that points to the URL of the image
      parentNodeId: node.id, // id of the parent node of the fileNode you are going to create
      createNode, // helper function in gatsby-node to generate the node
      createNodeId, // helper function in gatsby-node to generate the node id
      cache, // Gatsby's cache
      store // Gatsby's redux store
    });

    createNodeField({
      node,
      name: `featuredImg___NODE`,
      value: fileNode.id,
    });
    console.log(node);
  }
};

I found that the remote image (file) is created, it also has the parent.id of the original DatoCmsVideoItem. Also the output of console.log(node); contains the featuredImg___NODE key and value.

However I have no luck accessing the field featuredImg in GraphQL. I am not sure but it seems the createNodeField doesn't work on the node related to DatoCMS.

matjack1 commented 4 years ago

hey @winstonma I've tried to reproduce your issue and yes I cannot access the featuredImg in GraphiQL, but if I do something like this:

exports.onPostBootstrap = ({ getNodes }) => {
  const nodes = getNodes();

  nodes.forEach(node => {
    if (node.internal.type === `DatoCmsVideoItem`) {
      console.log(node.fields.featuredImg);
    }
  })
}

it works just fine (apart that there are some issues in your example, which I've simplified to add a plain field).

I'm going to comment also on the issue on the Gatsby repo. Let me know if you have any updates.

winstonma commented 4 years ago

@matjack1 You are totally correct.

As long as anyone can repeat the test case and confirm the problem, then it should be easy to trace the root cause.

Js-Brecht commented 4 years ago

@matjack1 responding to your question on the Gatsby thread:

should we remove that? We stop inferring because we had some compatibility issues and it was going to be deprecated in the future?

What's the recommended behaviour there? Thank you!

Can you point me to where it says type inferring is deprecated? I tried finding a blog post, issue log, and even scanned the code base, but couldn't find anything that said @infer or @dontinfer was going away.

I see that the noDefaultResolvers argument of the @infer directive is deprecated [ref]. They want you to add resolvers explicitly now, so that Gatsby doesn't have to determine what resolvers to attach to arbitrary field types... which makes sense to me. I never was a fan of having resolvers magically appear on fields; makes things easier to track when it's explicit, for one, and it's more efficient.

Type inferring is such an integral part of the schema generation, I'm not sure how they would remove it without causing an astronomical amount of work for every single plugin, and for every single end user.

matjack1 commented 4 years ago

@Js-Brecht you are right, sorry I was confused myself. Thank you so much for clarifying and pointing me in the right direction.

I think I've found a solution.

@winstonma I think you should use createResolvers as explained here: https://www.gatsbyjs.org/docs/schema-customization#extending-third-party-types

I've done this simple example that works for me:

exports.createResolvers = ({
  createResolvers
}) => {
  createResolvers({
    DatoCmsVideoItem: {
      featuredImg: {
        type: `String`,
        resolve(source, args, context, info) {
          return 'test';
        },
      },
    },
  })
}

Can you please confirm that works for you too?

@Js-Brecht does that make sense?

Js-Brecht commented 4 years ago

@matjack1 I think that would be the ideal method in this context.

To use @winstonma's example, you could also do something like this, since the resolve() function supports promises:

exports.createResolvers = ({
  createResolvers,
  store,
  cache,
  createNode,
  createNodeId,
  reporter
}) => {
  createResolvers({
    DatoCmsVideoItem: {
      featuredImg: {
        type: `File`,
        resolve(source, args, context, info) {
          return createRemoteFileNode({
            url: source.videoId,
            store,
            cache,
            createNode,
            createNodeId,
            reporter
          });
        },
      },
    },
  })
}

Quick question: how was type inferring breaking compatibility? Is that something specific to DatoCms data structure maybe?

winstonma commented 4 years ago

@Js-Brecht I need to modify the top part of your code slightly. Here is the modifiaction.

EDIT: Delete the query because so other people would not refer to the outdated query

Thanks again

Js-Brecht commented 4 years ago

@Js-Brecht I need to modify the top part of your code slightly. Here is the modifiaction.

You're totally right. I forget these things sometimes when I'm just winging it. Thanks for the correction!

winstonma commented 4 years ago

@matjack1 Thanks for this.

Is adding node fields in onCreateNode would not be the official way in the future release?

matjack1 commented 4 years ago

Thank you very much @Js-Brecht for participating in this issue, very appreciated!

@winstonma from Gatsby's documentation I think that createResolvers is the preferred way to handle this now and in the future as you'll always need to modify a schema set by a plugin.

I'll add some documentation about this in the readme.

I'm going to close this for now as I think we've found a solution, but please comment more if necessary.

winstonma commented 3 years ago

Hi @matjack1 I guess I saw a problem lately. I just tried to build the web site recently and saw the problem, I need to make the modification (the commented part) in order to get it work. It seems to me that source.videoId is missing (in fact, all attributes of source is missing). Here is the modified code:

EDIT: Delete the query because so other people would not refer to the outdated query

Not sure if it is just me or someone also face the problem.

matjack1 commented 3 years ago

hey @winstonma could you please send me the details of your project or the particular video that you are trying to access (maybe you can send the details at support@datocms.com to check if the problem is with your project or general).

winstonma commented 3 years ago

@matjack1 I created a fork of the gatsby-portfolio and added the createResolvers code. Here is the diff.

Then here is the test case

  1. Follow the basic procedure of gatsby-portfolio instruction, but of coz clone my repo instead (https://github.com/winstonma/gatsby-portfolio.git).
  2. Run yarn develop
  3. Open browser and go to GraphQL in-browser IDE (e.g. http://localhost:8000/___graphql)
  4. Paste the following Query and run
    query MyQuery {
    allDatoCmsWork {
    edges {
      node {
        title
        fullName
      }
    }
    }
    }

    Then you will see some thing print on the console. It is because console.log(source.entityPayload.attributes.title); print something on the console. However based on the conversation https://github.com/datocms/gatsby-source-datocms/issues/92#issuecomment-601869591, I would need console.log(source.title); instead (just like the source.videoId in my sample code).

Actually I didn't touch anything in DatoCMS and Gatsby, I just rebuild the whole project and suddenly source.title is gone.

matjack1 commented 3 years ago

hey @winstonma thank you, I would also need your project in Dato, could you please send that over as well?

winstonma commented 3 years ago

@matjack1 I didn't modify the content in DatoCMS. All I did is read the content from the DatoCMS.

matjack1 commented 3 years ago

@winstonma can you please share which DatoCMS project are you using so that I can double check your records and replicate the issue?

winstonma commented 3 years ago

@matjack1 I just leave a message to support@datocms.com

tbrannam commented 3 years ago

Think that @winstonma is seeing a similar issue as: https://github.com/datocms/gatsby-source-datocms/issues/128

stefanoverna commented 3 years ago

Hey, in recent versions, to achieve better loading performance, we had to make changes to how we store information inside the nodes, so yeah, the correct way now to read information is using source.entityPayload which returns exactly the JSON API response of our CMA with no transformations

winstonma commented 3 years ago

Thanks Stefano. I would modify my code according to the new changes.

winstonma commented 3 years ago

May I write down the updated code snippet in the final post so people could reference.

exports.createResolvers = ({
  actions,
  cache,
  createNodeId,
  createResolvers,
  store,
  reporter,
}) => {
  const { createNode } = actions;
  const resolvers = {
    DatoCmsVideoItem: {
      featuredImg: {
        type: `File`,
        resolve: async (source, args, context, info) => {
          return createRemoteFileNode({
            url: source.entityPayload.attributes.video_id,
            createNode,
            createNodeId,
            reporter,
            cache,
            store,
          });
        },
      },
    },
  };
  createResolvers(resolvers);
};

The query above will fetch a copy of the file (file URL specified in video_id) into your local build folder, using createRemoteFileNode, during build-time.

PatrykRudzinski commented 3 years ago

@winstonma Your solution works well, but I've faced one problem, it fetching all images during every build, so it's looks like cache don't work. Any ideas to resolve that?

winstonma commented 3 years ago

@PatrykRudzinski I am a free user of the datocms and I built a very small gatsby website so cache is not my priority. I guess @matjack1 would give you a perfect support on your question.

Just my two cent, I guess the caching mechanism, if there is one, should be done by Gatsby. This plugin, for my understanding, is to fetch the content from DatoCMS to your local drive for Gatsby to build.

Js-Brecht commented 3 years ago

@PatrykRudzinski the caching done by gatsby-source-filesystem is here and here

As you can see, when a file is received with a status of 200, it will be stored in Gatsby's .cache directory (in this case, .cache/caches/gatsby-source-filesystem). The headers from the file response will be stored in the node cache.

With headers cached, the next request for the same url will use If-None-Match: "<cached etag>". If the file has not changed, the server should respond with a 304. That means a network request is still going out, but it should not be downloading anything. It simply moves on, storing the cached file's path on the file node in Gatsby's GraphQL layer here

PatrykRudzinski commented 3 years ago

@winstonma thanks for quick reply. That's true, it's gatsby work to cache it. @Js-Brecht now everything is clear, thanks a lot.