Create new page object when running dirty pages

[okcoker]

So I ran into an issue a while ago related to object mutation within Gatsby. I don't remember exactly what it was other than it was related to pageContext. I was able to solve the previous problem by adding a couple Object.assign() calls within gatsby-node.js.

On my site, I create 2 different types of pages (using 2 different templates) within the createPages function using the allMarkdownRemark query seen in the blog starter example. Within the createPage action dispatcher call, I'm programmatically sending the next and previous page frontmatter object within the context object. When building the site, I can log out previousFrontmatter and nextFrontmatter within gatsby-node.js and everything seems correct until the page is rendered. Logging out the nextFrontmatter object within the page's render method outputs incorrectly. Oddly enough, I've only seen this only happen for one page of mine.

I narrowed down the location of where things seemed to be getting mutated for that page to a graphql call here. Before that line, logging out page.context.nextFrontmatter is correct and after that line, it's completely different. I'd imagine this issue has something to do with the spread operator being a shallow extend?

I'm not entirely sure of the exact problem here other than this change is working for me. I'm not sure if the other call sites for queryRunner might need the same treatment but I didn't wanna go overboard (although I think I only saw 3 call sites for it).

[KyleAMathews]

Deploy preview failed.

Built with commit 1955bfb

https://app.netlify.com/sites/using-styled-components/deploys/5977e3b5a700c42160ddc90e

[gatsbybot]

Deploy preview ready!

Built with commit 5e73963

https://deploy-preview-1537--gatsbyjs.netlify.com

[gatsbybot]

Deploy preview ready!

Built with commit 5e73963

https://deploy-preview-1537--using-drupal.netlify.com

[gatsbybot]

Deploy preview ready!

Built with commit 5e73963

https://deploy-preview-1537--gatsbygram.netlify.com

[KyleAMathews]

Hmm so weird, we shouldn't be mutating the page data anywhere... thanks for the investigation! Can you see anywhere in query-runner.js where there might be a mutation?

[okcoker]

@KyleAMathews So I'm not sure if its the graphql library itself, but here's what I noticed:

After the graphql call, page.context.nextFrontmatter is mutated.

[KyleAMathews]

Hmmm gotcha, very weird. Added this quick PR which could solve it. Want to get a release out tonight so will just roll with that. I appreciate that this PR solves the problem but it feels a bit blunt to me. I'd rather find the root cause and fix that. If #1553 doesn't change anything, let's keep investigating.

#1553

[okcoker]

Hey @KyleAMathews this PR does seem blunt to me as well but it might be a better band-aid than #1553. #1553 doesn't fix it because by the time it get's down to that line, the context was already mutated from the graphql call above it.

Also the spread operator won't deep clone nested objects within page.context so my page.context.nextFrontmatter object is the same reference before and after a { ...page.context } expression.

I also noticed changing that graphql line to result = await graphql(component.query, JSON.parse(JSON.stringify({ ...page, ...page.context }))) did not fix it (which I thought might be closer to the root cause) so now I suspect there is some sort of race condition going on with the debounced checkJobsDone (or findAndRunQueriesForDirtyPaths since that's where the JSON.parse(JSON.stringify(page)) fixed the issue.)

[KyleAMathews]

Ok, good points. Yeah feel free to revert #1553 then. Won't have time to investigate this issue for the next week or so but would love a PR if you do identify the root cause.

[okcoker]

I may have some time a little later. Can we reopen this PR, I'll add on to it

[okcoker]

Found out its this extractFieldExamples method that is mutating things. Since it's only using nodes[0] to infer types, that item for me has a context.nextFrontmatter of null since there's no next item.

So, I tried something for fun. If i default my context.nextFrontmatter and context.prevFrontmatter objects to {} instead of null within gatsby-node.js, I get the desired output.

[okcoker]

Ok so the problem is that _.mergeWith is mutative. You wouldn't have seen this issue with non nested deeply nested objects so I added the fix and tests.

[KyleAMathews]

Awesome! Nice debugging + fix!

We'll want to use something that handles circular references though. Ran into that a few weeks ago on a client site and had to make core safe for that.

Lodash's cloneDeep should work.

[okcoker]

@KyleAMathews I'm not sure why but changing that line to use _.cloneDeep gives an error when trying to build my site

building schemaUNHANDLED REJECTION Error: Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "0___resolve" does not.
    at assertValidName (/path/node_modules/graphql/utilities/assertValidName.js:44:11)
    at /path/node_modules/graphql/type/definition.js:632:42
    at Array.map (native)
    at defineEnumValues (/path/node_modules/graphql/type/definition.js:631:21)
    at new GraphQLEnumType (/path/node_modules/graphql/type/definition.js:549:20)
    at inferInputObjectStructureFromNodes (/path/node_modules/gatsby/dist/schema/infer-graphql-input-fields.js:224:22)
    at _callee$ (/path/node_modules/gatsby/dist/schema/build-node-types.js:92:36)
    at tryCatch (/path/node_modules/regenerator-runtime/runtime.js:65:40)
    at Generator.invoke [as _invoke] (/path/node_modules/regenerator-runtime/runtime.js:303:22)
    at Generator.prototype.(anonymous function) [as next] (/path/node_modules/regenerator-runtime/runtime.js:117:21)
    at step (/path/node_modules/babel-runtime/helpers/asyncToGenerator.js:17:30)
    at /path/node_modules/babel-runtime/helpers/asyncToGenerator.js:28:13
    at run (/path/node_modules/babel-runtime/node_modules/core-js/library/modules/es6.promise.js:87:22)
    at /path/node_modules/babel-runtime/node_modules/core-js/library/modules/es6.promise.js:100:28
    at flush (/path/node_modules/babel-runtime/node_modules/core-js/library/modules/_microtask.js:18:9)
    at _combinedTickCallback (internal/process/next_tick.js:67:7)

[okcoker]

Ah nvm I forgot to spread it. Pushed up the change.

[KyleAMathews]

Deploy preview failed.

Built with commit 1955bfb

https://app.netlify.com/sites/image-processing/deploys/5977e3b4a700c42160ddc908

[KyleAMathews]

Deploy preview failed.

Built with commit 1955bfb

https://app.netlify.com/sites/using-remark/deploys/5977e3b5a700c42160ddc914

[KyleAMathews]

Deploy preview failed.

Built with commit 1955bfb

https://app.netlify.com/sites/using-jss/deploys/5977e3b6a700c42160ddc929

[KyleAMathews]

Deploy preview failed.

Built with commit 1955bfb

https://app.netlify.com/sites/using-contentful/deploys/5977e3b5a700c42160ddc911

[KyleAMathews]

Deploy preview failed.

Built with commit 1955bfb

https://app.netlify.com/sites/using-glamor/deploys/5977e3b4a700c42160ddc90b

[KyleAMathews]

Deploy preview failed.

Built with commit 1955bfb

https://app.netlify.com/sites/using-postcss-sass/deploys/5977e3b4a700c42160ddc902

[KyleAMathews]

Thanks for the investigation and fix! This has probably been creating lots of weird bugs in other people's projects as well so great to get it cleaned up.