Guide plugin authors to write source plugins that correctly interact with the cache #31214

johno · 2019-02-15T01:20:52Z

johno
Feb 15, 2019

This is related to #11747 as an isolated, reproducible case of caching issues. cc/ @pieh @DSchau

Description

Numerous plugins in the ecosystem aren't interlinking a node and the parent on onCreateNode which causes nodes in the cache to suddenly go missing. I stumbled across this issue working on a proof of concept with gatsby-source-instagram-all but I'm sure this affects plenty of other plugins.

It's not currently intuitive how to properly set up data sourcing in ways that interact well with the cache.

These issues are unfortunate because they create distrust in the cache (rightfully so). Logging warnings and helping to guide plugin authors towards the right path will help alleviate caching woes.

Steps to reproduce

Use a plugin like the reproduction and stop, then start, the server.

Expected result

Restarting the server and running the build should work.

Actual result

Restarting the server and running the build require clearing the cache.

Steps to fix

Currently, the steps to fix involves rm -rf .cache 🙀.

To solve the fundamental issue here will require two steps (I think)

• We should issue a warning in onCreateNode if there's no node passed as a parent and link to a doc that elaborates on the issue
• We should have a doc for plugin authors that describe the Gatsby caching system and how nodes need to be interlinked to be properly cached and avoid errors

I'm anticipating that the doc should be in the style of a guide for plugin authors that might be sourcing nodes since the existing docs (that I can find) are more low-level but I'll defer to @marcysutton here.

Screenshot transformer does that - in very similar way as linked plugin (because both use createRemoteFileNode that doesn't set parent). The difference is screenshot plugin also touches those file nodes so they don't get garbage collected. We definitely need to add ability to set parent to createRemoteFileNode helper.

0 replies

KyleAMathews · 2019-02-15T01:53:19Z

KyleAMathews
Feb 15, 2019

Ah yeah, ok so it's a weakness in the helper. Oh and we also shouldn't then delete nodes that are linked to from other nodes 🤔

That's the real problem. File nodes don't need a parent. They just need not deleted. If I'm remembering the stale node algorithm correctly, we only check for parents.

0 replies

KyleAMathews · 2019-02-15T02:15:49Z

KyleAMathews
Feb 15, 2019

This is a pretty major bug. The stale collection stuff is our GC step but since we're not collecting all references, we're deleting too much.

https://github.com/gatsbyjs/gatsby/blob/7d31fe75f8d73bb52e7f10384033429bafbc8186/packages/gatsby/src/utils/source-nodes.js

I'm not sure though how to cheaply find ___NODE references since recursing through every node would be very expensive.

0 replies

pieh · 2019-02-15T02:25:10Z

pieh
Feb 15, 2019
Maintainer

I'm not sure though how to cheaply find ___NODE references since recursing through every node would be very expensive.

Not ideal, but we do recurse through each node when we produce example value for schema inference. But using that has potential to easily introduce regressions when changing seemingly unrelated code.

I honestly am not sure we should do that (shield ___NODE linked nodes from being garbage collected) - especially with onCreateNode callback that is really uniquely fitting for transformer plugins. The file node that is created and linked there is really derived from "parent" node so it makes sense to enforce that this File node is child.

I guess maybe instead of changing createRemoteFileNode to add support for setting parent, we could adjust createParentChildLink action to set both children on parent, and parent on child?

gatsby/packages/gatsby/src/redux/actions.js

Lines 760 to 773 in e651e9c

    
           actions.createParentChildLink = ( 
        
             { parent, child }: { parent: any, child: any }, 
        
             plugin?: Plugin 
        
           ) => { 
        
             // Update parent 
        
             parent.children.push(child.id) 
        
             parent.children = _.uniq(parent.children) 
        
             return { 
        
               type: `ADD_CHILD_NODE_TO_PARENT_NODE`, 
        
               plugin, 
        
               payload: parent, 
        
             } 
        
           }

That + warning/error containing instructions to use that action seems like it would solve this without adding overhead of recursing nodes to find ___NODE fields

0 replies

KyleAMathews · 2019-02-15T02:38:33Z

KyleAMathews
Feb 15, 2019

Hmm thought of a cheap way to do the fix. Just track which nodes are created in onCreateNode from which node. This would be an implicit parent/child relationship even if the data wasn't modeled that way. We could wrap the createNode action to save this. Then in the GC step, we could do a deep search on the remaining parent nodes for links.

0 replies

sidharthachatterjee · 2019-02-20T18:31:58Z

sidharthachatterjee
Feb 20, 2019

So I just tried to reproduce this with gatsby-source-instagram-all and can't seem to be able to. It is consistently working for me.

redux-state.json is written in .cache correctly and onCreateNode is being called everytime the server is started.

Any ideas?

0 replies

sidharthachatterjee · 2019-02-20T19:39:26Z

sidharthachatterjee
Feb 20, 2019

Turns out the Instagram API returns tags for an image in random order every time. So content digest is different every time! That's why they were getting marked as dirty and couldn't reproduce.

0 replies

sidharthachatterjee · 2019-02-20T19:57:32Z

sidharthachatterjee
Feb 20, 2019

Hmm thought of a cheap way to do the fix. Just track which nodes are created in onCreateNode from which node. This would be an implicit parent/child relationship even if the data wasn't modeled that way. We could wrap the createNode action to save this. Then in the GC step, we could do a deep search on the remaining parent nodes for links.

This was published in

- [email protected]
 - [email protected]
 - [email protected]
 - [email protected]
 - [email protected]
 - [email protected]
 - [email protected]

0 replies

johno · 2019-02-20T20:52:29Z

johno
Feb 20, 2019
Author

Going to close this since rather than guiding we've been able to fix it with code itself. Thanks @KyleAMathews, @pieh and @sidharthachatterjee!

0 replies

pieh · 2019-04-01T13:54:07Z

pieh
Apr 1, 2019
Maintainer

Reopening this, because this was fixed in single instance (createRemoteFileNode) and not generally, so users can still hit this - as demonstrated in #12981

0 replies

2019-04-22T17:27:35Z

gatsbot[bot]
bot Apr 22, 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! 💪💜

0 replies

smerth · 2019-06-08T03:18:47Z

smerth
Jun 8, 2019

Hi there,

I have an issue and I think it might fit here. I wrote a plugin called gatsby-source-gh-readme, which you can find on NPM gatsby-source-gh-readme and the repo is on github here gatsby-source-gh-readme.

The plugin makes a graphql call to the github graphql api and should pull in the file contents of the README.md file at the root of each project in a github user's account and then create a node (of mediaType: "text/markdown") for each in Gatsby.

The plugin works, for each README.md file Gatsby creates a node of type markdown and then those node are processed by the remark plugins just like the blog post markdown files...

However, frequently (almost every time,) I need to clear the cache before running gatsby develop or the build will fail. If I clear the cache prior to running gatsby develop the build succeeds.

I prepare the data for createNode() with this function:

// Helper function that processes a repository node to match Gatsby's node structure
  const processRepo = repo => {
    const nodeId = createNodeId(`repo-readme-${repo.id}`);
    const readme = repo.readme;
    const nodeData = Object.assign({}, repo, {
      id: `repo-readme-${nodeId}`,
      parent: null,
      children: [],
      internal: {
        mediaType: "text/markdown",
        type: `GithubReadme`,
        content: readme,
        contentDigest: createContentDigest(repo)
      }
    });
    return nodeData;
  };

you can see that parent is null and children is an empty array. I don't understand what parent should be, or indeed what children should be…

Apart from the fact that there is probably a better way to write this plugin, it does seem to be example of what your talking about in this issue.

Apart from flagging this as a potential example of the problem discussed in this issue. I have some general questions about this plugin that are not related:

when writing a plugin how do I decide what parent, or child should be?
how could I write this to make use of graphql from gatsby itself, it seems silly to use another library to make the call to the github api, but I couldn't find a way to use gatsby's graphql from inside the plugin.
how can I get feedback on how I wrote this plugin? I am sure there is a better way to write this...

0 replies

maiertech · 2020-10-27T00:49:26Z

maiertech
Oct 27, 2020

Here is an example of a theme that creates tag nodes that disappear from the cache for no obvious reason: reflexjs/reflexjs#39.

0 replies

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Guide plugin authors to write source plugins that correctly interact with the cache #31214

{{title}}

Replies: 15 comments

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

{{title}}

{{title}}

Select a reply

Guide plugin authors to write source plugins that correctly interact with the cache #31214

Description

Steps to reproduce

Expected result

Actual result

Steps to fix

To solve the fundamental issue here will require two steps (I think)

Related

Replies: 15 comments

pieh Feb 15, 2019 Maintainer

pieh Feb 15, 2019 Maintainer

johno Feb 20, 2019 Author

pieh Apr 1, 2019 Maintainer

gatsbot[bot] bot Apr 22, 2019

pieh
Feb 15, 2019
Maintainer

pieh
Feb 15, 2019
Maintainer

johno
Feb 20, 2019
Author

pieh
Apr 1, 2019
Maintainer

gatsbot[bot]
bot Apr 22, 2019