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

pageContext is not updated when context change #11691

Closed
RageYL opened this issue Feb 11, 2019 · 11 comments
Closed

pageContext is not updated when context change #11691

RageYL opened this issue Feb 11, 2019 · 11 comments
Labels
type: bug An issue or pull request relating to a bug in Gatsby

Comments

@RageYL
Copy link

RageYL commented Feb 11, 2019

Hi,

The issue is not as straightforward as i thought it would be, so here is a repo to reproduce it: https://github.com/RageYL/gatsby-pageContext

And this is different from #6097

I use context from createPage to pass data to a template (I started using gatsby 3 days ago, so not 100% sure if this is the good way), but when the data is updated the pageContext value is not updated.
The pageContext is not updated if:

  • there is a query using MarkdownRemark (i tried without query or with sitePage and no problem)

If you confirm this is indeed an issue, i'm ready to dig in and submit a pull request.

@RageYL RageYL changed the title page note refreshed when pageContext updated pageContext is not updated when context change Feb 11, 2019
@jonniebigodes jonniebigodes added the type: question or discussion Issue discussing or asking a question about Gatsby label Feb 11, 2019
@jonniebigodes
Copy link

@RageYL i've picked up on your issue, i've cloned your repo into my machine installed it and did some tests, namelly:

  • Created a json object with a similar structure and provided the data from the json file.
  • Updated the template to implemement the livecycle method componentDidUpdate and getDerivedStateFromProps to see if it could solve the issue.
  • Passed not the object but the property to pageContext, to see if produced an update.

As you can see bellow, the value of datafromJson on the right, the devtools is x and on the left is b
noupdate

My reasoning for this behaviour is that as is, the api hook, either with your object or mine, is on a "fire and forget" mode, it fetches the data once, passes it along and doesn't care about subsequent changes. What you could probably do is on gatsby.node.js filem is the following, you add a watcher to the file to see if it has changed, and if so, delete the page and recreate it. I have not tested this And it's not tested.

Also three days into Gatsby and already into one api that people tend to struggle more. Congratulations are in order.
Feel free to provide feedback ,so that we can close this issue

@jonniebigodes jonniebigodes added the status: awaiting author response Additional information has been requested from the author label Feb 11, 2019
@RageYL
Copy link
Author

RageYL commented Feb 11, 2019

What i find very strange is that everything works fine if there is no query or if i change the query's content (maybe something related to MarkdownRemark resolvers and the cache).
I updated my gatsby-node.js to use createPageStatefully (better choice indeed as my data is not managed by gatsby): https://github.com/RageYL/gatsby-pageContext/blob/createPagesStatefully/gatsby-node.js

Now if i update the data and restart gatsby the change is reflected on the page, but when i update without restarting gatsby, the page still display the same value. So it's already better, but it seems there is still something wrong with the cache.

@pieh
Copy link
Contributor

pieh commented Feb 11, 2019

Ok, this is bit problematic. There was related fix some time ago - #11048 (which is totally fine).

But reason it doesn't work for this use case is that we don't check page contexts between runs (and this is not quite easy to do right now, but doable). In createPage we could first check if we already have that page in memory store - if we do continue as is, if not we would want to load cached query results for that page (if that exists) - just to grab context used for that, and then we could compare and set contextModified flag on action which will trigger query re-run

@pieh
Copy link
Contributor

pieh commented Feb 11, 2019

I didn't notice you commented while I was writing previous comment

What i find very strange is that everything works fine if there is no query or if i change the query's content (maybe something related to MarkdownRemark resolvers and the cache).

For the most part this is because we try to cache our query results so subsequent runs don't do extra work and re-use cached results. But there are cases where we don't invalidate our cache and you hit one of those.

@RageYL
Copy link
Author

RageYL commented Feb 11, 2019

Hum, i think i managed to make it work with createPagesStatefully.

When the file changed i was deleting the page and then creating it again.
But by reading redux/actions.js, if the page is deleted, oldPage is undefined and contextModified is set to false.
By not deleting the page, contextModified is set to true and the page is refreshed correctly.

So it seems to work correctly with createPagesStatefully: https://github.com/RageYL/gatsby-pageContext/tree/createPagesStatefully.

Edit: thinking about it again, it means that when we call deletePage, the page is not removed from the cache ?

Edit2: I just tried what i had on my mind yesterday and it seems to fix the problem:

in redux/actions.js:

const contextModified =

  - const contextModified = !!oldPage && !_.isEqual(oldPage.context, internalPage.context);
  + const contextModified = !oldPage || !_.isEqual(oldPage.context, internalPage.context);

I don't have enough experience to know what this update would break. But at least it seems to me that between run, the context is never modified as oldPage is always undefined the first time. And it seems to be the reason the page is not refreshed.

@pieh
Copy link
Contributor

pieh commented Feb 12, 2019

@RageYL this change would cause gatsby to always rerun all queries in bootstrap.
What we probably need is to purge node dependencies for page when you delete - in here https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/src/redux/reducers/component-data-dependencies.js we would need to handle DELETE_PAGE action and remove node/connection dependencies for given page. Then we you re-create page our query runner will run query for it because the path doesn't yet have any dependencies ( https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/src/internal-plugins/query-runner/page-query-runner.js#L43-L45 )

@pieh pieh added type: bug An issue or pull request relating to a bug in Gatsby and removed status: awaiting author response Additional information has been requested from the author labels Feb 12, 2019
@RageYL
Copy link
Author

RageYL commented Feb 12, 2019

I think your solution may not work in certain situations: in the first case I
presented, the page is created once with createPage. The server had to be
reloaded manually for the change to take effect. In this case the next time
gatsby is run, createPage would be called again, but deletePage would never be
called and thus the page would not be re-evaluated (this could also happen if
there is a watcher calling deletePage when the file is modified: if the file is
modified while gatsby is not running, the deletePage would never be called).

Just some thoughts =)

Edit: I just removed some statements, this could be solved by what you describe in your first message.

Edit 2: after some digging I found one way to solve the problem i just mentioned (i didn't touch anything related to the deletePage)

- const contextModified = !!oldPage && !_.isEqual(oldPage.context, internalPage.context);

+ let contextModified = !!oldPage && !_.isEqual(oldPage.context, internalPage.context);
+ 
+ if (oldPage == null)
+ {
+ 	let program  = store.getState().program
+ 	let dataPath = store.getState().jsonDataPaths[internalPage.jsonName]
+ 
+ 	try
+ 	{
+ 		let name = path.join(program.directory, `public`, `static`, `d`, dataPath + `.json`)
+ 		let data = JSON.parse(fs.readFileSync(name))
+ 
+ 		contextModified = !_.isEqual(data.pageContext, internalPage.context)
+ 
+ 		console.log(contextModified)
+ 	}
+ 	catch (e)
+ 	{
+ 		console.log(e)
+ 	}
+ }

So I'll stop my digging here :D. I don't know how i can help more about this. If you want my help for something related to this issue, just ask =)

@RageYL
Copy link
Author

RageYL commented Feb 13, 2019

I'm not sure if i'm allowed to do that, but as yesterday's last edit may provide a solution for the problem, i'm writting a new comment to be sure you get notified.

@gatsbot
Copy link

gatsbot bot commented Mar 6, 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! 💪💜

@gatsbot gatsbot bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Mar 6, 2019
@gatsbot
Copy link

gatsbot bot commented Mar 17, 2019

Hey again!

It’s been 30 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.

Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to reopen this issue or create a new one if you need anything else.

Thanks again for being part of the Gatsby community!

@LekoArts
Copy link
Contributor

This was fixed in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

No branches or pull requests

8 participants