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(gatsby-transformer-remark): correctly pass cache to sub plugins #10892

Merged
merged 8 commits into from
Jan 8, 2019

Conversation

DSchau
Copy link
Contributor

@DSchau DSchau commented Jan 7, 2019

Description

Separate some changes from #10146, specifically so we can make this a non-breaking change if the right version of Gatsby isn't specified.

Notes:

  • I will add a warning/log (or error?) if the API doesn't exist so the user can update.
  • This should probably be a minor version bump, even if it's not breaking

Related Issues

Related to #10146

@@ -58,6 +58,26 @@ const tableOfContentsCacheKey = node =>
const withPathPrefix = (url, pathPrefix) =>
(pathPrefix + url).replace(/\/\//, `/`)

// TODO: remove this check with next major release
// the getCache feature was implemented in gatsby@2.0.88
const safeGetCache = ({ getCache, cache, reporter }) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pieh this warning looks like:

success onPreBootstrap — 0.004 s
success source and transform nodes — 0.072 s
success building schema — 0.175 s
success createPages — 0.030 s
success createPagesStatefully — 0.021 s
success onPreExtractQueries — 0.003 s
success update schema — 0.102 s
success extract queries from components — 0.119 s
warning It looks like you're using a slightly outdated version of gatsby with gatsby-transformer-remark.
To update, run 'npm install gatsby@^2.0.86 --save' or 'yarn add gatsby@^2.0.86'

I thought about using onPreBootstrap but I like how we can remove this code all in one spot (easier refactoring). Up for whatever though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also it'd be cool to think more broadly about how we introduce non-backwards compatible API changes, and if we could improve this.

For now this will work fine.

@@ -4,7 +4,7 @@
"version": "1.0.0",
"author": "Dustin Schau <dustin@gatsbyjs.com>",
"dependencies": {
"gatsby": "^2.0.71",
"gatsby": "^2.0.88",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These don't really matter. Just for local stuff!

@@ -5,7 +5,7 @@ module.exports = function remarkPlugin({ cache, markdownAST }) {
visit(markdownAST, `html`, async node => {
if (node.value.match(id)) {
const value = await cache.get(id)
node.value = node.value.replace(/%SUBCACHE_VALUE%/, () => `>${value}<`)
node.value = node.value.replace(/%SUBCACHE_VALUE%/, value)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

60% less jank ™️

return cache
}
return getCache(id)
const safeGetCache = ({ getCache, cache }) => id => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for posterity, we already warn on conflicting peerDeps, so a second warning would be sorta redundant.

Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

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

🌮

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