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

fix(develop): emit stale page data messages when staticQueryHashes change #28349

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Nov 29, 2020

Description

Currently we don't reprocess page-data's staticQueryHashes array in develop when those change, which means if we have cached page-data we will not handle fetching any additions to that array. If the new static queries are not cached already this results in "Cannot fetch static query result", which can be fixed by refresh (but is bad experience of course).

This will mark cached pageData entries as stale which will cause dev runtime to refetch it next time loadPage is called instead of using (now stale) cached result and run all the regular processing (which will fetch static queries that are not fetched/cached yet).

This is not complete fix however. It doesn't exactly fix the problem when staticQueryHashes changed for a page user currently view in the browser ( note TODO comments I added in added e2e test scenario and dev-loader ), but it handles at least some cases.

Note - I initially thought this is query on demand specific problem, but it's not - it's generic problem (so changes in loader.js to refetch stale data had to be expanded to gatsby develop when not using query on demand)

The e2e test file changes are best checked with "Hide whitespace changes" ( https://github.com/gatsbyjs/gatsby/pull/28349/files?w=1 ) because I just did wrap existing test with additional cy.describe to mark that those are due to data updates and new block cover case of adding static query to page-data (which originates from source code changes and not data changes)

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Nov 29, 2020
@pieh pieh added topic: resource loading* and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Nov 29, 2020
@KyleAMathews
Copy link
Contributor

Yeeeessss 🙏

Copy link
Contributor

@sidharthachatterjee sidharthachatterjee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch @pieh

@sidharthachatterjee sidharthachatterjee added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Nov 30, 2020
@gatsbybot gatsbybot merged commit 5096e90 into master Nov 30, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix/invalidate-cached-page-queries-when-staticqueryhashes-change branch November 30, 2020 17:25
pieh added a commit that referenced this pull request Nov 30, 2020
pieh added a commit that referenced this pull request Dec 1, 2020
…ange (#28349) (#28391)

(cherry picked from commit 5096e90)

Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
@alacret
Copy link

alacret commented Dec 31, 2020

@KyleAMathews @sidharthachatterjee @pieh

Any idea how to patch this? or avoid this to happen in the meantime a permanent solution gets implemented? With this new version, I am still experiencing the issue when the browser caches the site.

We started experiencing this when we heavily start using StaticQuery, but we missed the bug until the full site was finished. We heavily use Static Query for pulling the .json data with Image paths.

I've also tried that AWS Amplify sent no-cache instructions for the Cache-control to .json .js files and I've also tried to prevent any file from being cached without any luck.

The only thing that I have left to do is remove the use of the StaticQuery + JSON data source plugin and find a different way of optimizing the images, maybe with the new Image component.

Screen Shot 2020-12-31 at 11 24 02 AM

I'll appreciate any help as I don't want to go through all the trouble of removing the use of JSON files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants