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

feat(gatsby): Add new caching clearing behavior for webpack/file downloads behind flags #28334

Merged
merged 11 commits into from
Dec 1, 2020

Conversation

KyleAMathews
Copy link
Contributor

@KyleAMathews KyleAMathews commented Nov 27, 2020

We're investigating ways to be smarter about clearing caches.

Currently we delete the entire cache when your gatsby-node.js, gatsby-config.js, or package versions change or you run gatsby clean. This is unnecessarily aggressive — changing these things don't invalidate most caches. The current cache clearing behavior means very expensive operations like downloading and processing images have to be repeated quite often which makes iterating on the website a lot more frustrating than it should.

This PR adds two new flags:

  • PRESERVE_FILE_DOWNLOAD_CACHE prevents file downloads from being deleted during cache clearing events (other than gatsby clean which still deletes the entire cache).
  • PRESERVE_WEBPACK_CACHE prevents the webpack cache from deleted with the same caveat for gatsby clean

We will continue to clear other plugin caches e.g. those created by source & transformer plugins. We're looking at ways to safely making more selective clearing those caches as well.

To enable the new cache behavior, add to your gatsby-config.js:

module.exports = {
  flags: {
    PRESERVE_FILE_DOWNLOAD_CACHE: true,
    PRESERVE_WEBPACK_CACHE: true
  },
  plugins: [...]
}

These will also be enabled if you add the FAST_DEV flag which enables a number of WIP ways to speed up the dev server.

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Nov 27, 2020
@KyleAMathews KyleAMathews added impact: high and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Nov 27, 2020
@KyleAMathews
Copy link
Contributor Author

KyleAMathews commented Nov 28, 2020

Hmmm is this even a breaking change? Why not just release this now? Not behind a flag?

I think I started with this being a flag in mind as I also want to change the behavior of gatsby clean to let people clean only parts of the cache and steer them away from clearing expensive parts of the cache that aren't probably causing trouble — this would be a breaking change but what this PR ended up being doesn't seem like it is.

@wardpeet
Copy link
Contributor

Technically this can end up with issues. Unsure if cache keys of webpack & babel are correct. If we change gatsby-source-filesystem we could end up in a weird state. Do I think this is likely, no but better safe than sorry.

@KyleAMathews KyleAMathews changed the title feat(gatsby): Add new caching behavior for v3 behind a flag feat(gatsby): Add new caching clearing behavior for webpack/file downloads behind flags Nov 30, 2020
@KyleAMathews
Copy link
Contributor Author

Some updates from a conversation between @pieh, @wardpeet, and me earlier

  • this isn't a breaking change per se but we need more testing to be sure we're covering our basis before shipping this to everyone so keep it behind a flag
  • we need to fix caching for realz for createRemoteFileNode with a new jobs API
  • add semver check for invite to try file downloads flag so we don't keep inviting people after we've fixed this in gatsby-source-filesystem

// Attempt to empty dir if remove fails,
// like when directory is mount point
await fs.remove(cacheDirectory).catch(() => fs.emptyDir(cacheDirectory))
// Comment out inviet until we can test perf impact
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Comment out inviet until we can test perf impact
// Comment out invite until we can test perf impact

;)

Choose a reason for hiding this comment

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

;)

Choose a reason for hiding this comment

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

How

@pieh pieh merged commit f57d590 into master Dec 1, 2020
@delete-merged-branch delete-merged-branch bot deleted the v3-cache branch December 1, 2020 22:44
pieh pushed a commit that referenced this pull request Dec 1, 2020
…loads behind flags (#28334)

* feat(gatsby): Add new caching behavior for v3 behind a flag

* change to CACHE_CLEAR

* cleanup

* Split into two flags

* Update invite wording + do semver check so we don't keep showing this to updated versions of source-filesystem

* Add invite for webpack flag

* First try grabbing the plugin from  and if that fails, try requiring

* Don't use del or readdir until measure perf more

* Remove now unused imports

* Nest ifs

(cherry picked from commit f57d590)
pieh pushed a commit that referenced this pull request Dec 1, 2020
…loads behind flags (#28334) (#28425)

* feat(gatsby): Add new caching behavior for v3 behind a flag

* change to CACHE_CLEAR

* cleanup

* Split into two flags

* Update invite wording + do semver check so we don't keep showing this to updated versions of source-filesystem

* Add invite for webpack flag

* First try grabbing the plugin from  and if that fails, try requiring

* Don't use del or readdir until measure perf more

* Remove now unused imports

* Nest ifs

(cherry picked from commit f57d590)

Co-authored-by: Kyle Mathews <[email protected]>
fbuireu added a commit to fbuireu/biancafiore that referenced this pull request Dec 22, 2020
fbuireu added a commit to fbuireu/biancafiore that referenced this pull request Dec 22, 2020
Copy link

@SirJosh1987 SirJosh1987 left a comment

Choose a reason for hiding this comment

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

Like this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants