feat(gatsby): provide a mechanism for plugins to use a named cache instance

[DSchau]

Fixes #8788

This PR provides a mechanism for any plugin to call a getCache helper, which takes a single argument (name) and returns or creates a new cache instance for that named instance.

This is particularly applicable to cases where a plugin has sub-plugins (e.g. gatsby-transformer-remark). In this scenario, we have a cache mismatch, essentially the cache provided by Gatsby is named to the plugins name, whereas the cache provided by gatsby-transformer-remark is named gatsby-transformer-remark

This PR fixes this imbalance. Few questions/comments:

• I think it might make sense to override the cache passed, e.g. cache: getCache(plugin.name) for sub plugins. We can still inject getCache so if the parent cache is needed (rarely?) this will still function, as expected
• I'll need to bump the peer dependency of gatsby-transformer-remark to the next version of gatsby at merge time

[DSchau]

Wasn't used

[felipesere]

@DSchau I think CI failed due flatmap-stream was yanked (or at least the version that was used somewhere transitively) and should be ok now? I ran the tests locally and they are happy 😄

[pieh]

I wonder if doing something like:

cache: getCache(plugin.id)

to pass individual cache for subplugin woudln't make sense, so subplugins don't need to use getCache directly, and would automatically get same cache instance as in their gatsby-node API hooks?

Questions:

• Is there scenario where it is actually desirable that subplugin use parent plugin cache?
  I can't think if reason for this of top of my head.
• Will this break any subplugin?
  Cache key generating functions (

  gatsby/packages/gatsby-transformer-remark/src/extend-node-type.js

  Lines 31 to 50 in 6a8c593

  	const astCacheKey = node =>
  	`transformer-remark-markdown-ast-${
  	node.internal.contentDigest
  	}-${pluginsCacheStr}-${pathPrefixCacheStr}`
  	const htmlCacheKey = node =>
  	`transformer-remark-markdown-html-${
  	node.internal.contentDigest
  	}-${pluginsCacheStr}-${pathPrefixCacheStr}`
  	const htmlAstCacheKey = node =>
  	`transformer-remark-markdown-html-ast-${
  	node.internal.contentDigest
  	}-${pluginsCacheStr}-${pathPrefixCacheStr}`
  	const headingsCacheKey = node =>
  	`transformer-remark-markdown-headings-${
  	node.internal.contentDigest
  	}-${pluginsCacheStr}-${pathPrefixCacheStr}`
  	const tableOfContentsCacheKey = node =>
  	`transformer-remark-markdown-toc-${
  	node.internal.contentDigest
  	}-${pluginsCacheStr}-${pathPrefixCacheStr}`

  ) are not exported, so shouldn't be considered as public API, so this shouldn't be considered as breaking change.

[pieh]

Ultimately I would like to prep helper functions/objects in core, so parent plugin could pass prepped helpers, but this is out of scope for this PR I think

[raae]

Is anyone looking into this @pieh?

[DSchau]

@pieh I provided a relatively janky e2e test to validate this functionality in 307b49e. Not in love with it, but it is a nice way to validate this--open to suggestions or just removing it.

One thought I had here is that this is actually a breaking change for gatsby-transformer-remark as currently authored. getCache will not be passed to plugins in versions of gatsby < the version published here, so I think I'd propose the following:

• Rip out gatsby-transformer-remark stuff from this PR
• Publish this PR, largely just exposing the API to plugins
• Bump to a minor (or major?) release of gatsby-transformer-remark
• Add gatsby@the version published here as a peerDependency

Alternatively, I can write a little utility that will guard against getCache not existing, and fall back to the parent cache, e.g.

const getSubPluginCache = (plugin, { getCache, cache }) => {
  if (getCache) {
    return getCache(plugin.name);
  }
  return cache
}

// later

cache: getSubPluginCache(plugin, { getCache, cache })

Anything I'm missing? Sound reasonable?

[pieh]

@pieh I provided a relatively janky e2e test to validate this functionality in 307b49e. Not in love with it, but it is a nice way to validate this--open to suggestions or just removing it.

the >< replacement might seem a bit whacky - but it works, so all is good.

As for peerDeps bump - splitting PR seems reasonable. Don't tell anyone but when I was bit lazy in situation like this before I was manually bumping peerDeps to version that will yet be released just before merging and publishing instead of splitting PR.

Alternatively, I can write a little utility that will guard against getCache not existing, and fall back to the parent cache, e.g.

I think we should do both and even possibly warn user to update gatsby version if getCache doesn't exist.

[DSchau]

@pieh lol I almost proposed guessing what the peerDep would be 😆

I'll get this ship shape and split them apart and add in a little utility.

[DSchau]

@pieh want to take another look at this today?

[pieh]

👍