feat(gatsby-transformer-remark): add excerptAst to be exported as a GraphQL field #11237

chitoku-k · 2019-01-23T18:35:51Z

Description

This PR adds/refactors excerpt + excerptAst (which is a new field) because I personally found it incovenient gatsby-transformer-remark not exporting an excerpt field in AST. What I did was basically refactoring by splitting out the resolver of excerpt that was internally converting ASTs to HTML to two functions that have similar signatures to the getHtml() and getHtmlAst(), with some caching feature as well. The tests have been added for these ASTs.

Documentation could be added something like this:

diff --git a/packages/gatsby-transformer-remark/README.md b/packages/gatsby-transformer-remark/README.md
index 423329b..36a0d59 100644
--- a/packages/gatsby-transformer-remark/README.md
+++ b/packages/gatsby-transformer-remark/README.md
@@ -119,6 +119,7 @@ By default, excerpts have a maximum length of 140 characters. You can change the
       node {
         html
         excerpt(pruneLength: 500)
+        excerptAst(pruneLength: 500)
       }
     }
   }
@@ -178,14 +181,15 @@ Any file that does not have the given `excerpt_separator` will fall back to the

 ### Excerpts for non-latin languages

-By default, `excerpt` uses `underscore.string/prune` which doesn't handle non-latin characters ([https://github.com/epeli/underscore.string/issues/418](https://github.com/epeli/underscore.string/issues/418)).
+By default, `excerpt` and `excerptAst` use `underscore.string/prune` which doesn't handle non-latin characters ([https://github.com/epeli/underscore.string/issues/418](https://github.com/epeli/underscore.string/issues/418)).

-If that is the case, you can set `truncate` option on `excerpt` field, like:
+If that is the case, you can set `truncate` option on `excerpt` and `excerptAst` field, like:

 ```graphql
 {
   markdownRemark {
     excerpt(truncate: true)
+    excerptAst(truncate: true)
   }
 }

chitoku-k · 2019-01-24T05:12:20Z

@DSchau Could you help me find why this test sub-plugin caching fails while the others pass? I dug into an actual problem that caused the test to fail but got no luck with it. Thanks in advance.

chitoku-k · 2019-01-24T16:53:47Z

I have finally figured out that the test added in #10892 did not handle Promises that are passed to visit. Each Promise must be wrapped by Promise.all() so that the gatsby-transformer-remark knows the sub-plugins finished their procedures; otherwise, it can easily break with such an unrelated change.

DSchau · 2019-01-24T18:12:30Z

@chitoku-k so this PR seems to make sense in general. If we have htmlAST we should also expose an excerptAST similarly.

However - if you don't mind me asking, apart from being consistent here, what is this PR solving?

chitoku-k · 2019-01-24T18:29:58Z

@DSchau My project currently tries to utilize gatsby-remark-component that requires the AST be passed to its compiler. It works fine so far with htmlAsts, but the excerpts had to be manually generated using the same logic. This PR solves this problem rather than making consistency here.
Thanks.

DSchau

This looks good! Left a small comment.

Nice catch on the async issue, as well!

DSchau · 2019-01-24T17:58:17Z

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

+  `transformer-remark-markdown-excerpt-${format}-${
+    node.internal.contentDigest
+  }-${pluginsCacheStr}-${pathPrefixCacheStr}`
+const excerptAstCacheKey = node =>


Couldn't the excerptCacheKey be used with a format? Instead of two of the same function basically.

Yes, does it look better?

chitoku-k · 2019-02-21T17:08:01Z

@DSchau Any further thoughts for this?

DSchau · 2019-02-21T17:08:51Z

@chitoku-k checking it out now. Thanks for the reminder!

DSchau · 2019-02-21T20:30:20Z

I took a look! It looks like the arguments passed to excerpt (e.g. pruneLength) do not seem to be taking effect.

For instance, if we consider the following query, we'd expect the excerpt to be 1 character, as well as the excerpt provided in the AST should be 5 characters.

{
  allMarkdownRemark(limit:1) {
    edges {
      node {
        excerpt(pruneLength:1)
        excerptAst(pruneLength:5)
      }
    }
  }
}

This, unfortunately does not seem to be the case! I can dive in in a bit, but I don't believe this to be in a mergeable state quite yet. Would you be able to validate on your end that you're seeing the same?

For the record - I used gatsbyjs.org to test!

For posterity, here's the result of the above GraphQL query.

chitoku-k · 2019-02-23T08:51:02Z

@DSchau Thank you for reviewing and detailed description for the bug!
It turned out that the parameterized fields can never been cached without parameterized cache keys, as any later changes would no longer take effect. There should be chances that these parameters can be included to cache key, but I removed them because it didn't sound good choice to me.

…aphQL field

… as well partially reverting gatsbyjs#11363

chitoku-k · 2019-02-23T09:16:34Z

Fixed the regression and rebased to master (due to failures on e2e tests).

DSchau · 2019-02-25T15:49:35Z

@chitoku-k thank you! I'll take one more look at this, and then hopefully get it merged and published today. Thank you for your patience, and for fixing the issues as they arose!

DSchau

Doing some final validations now, but I think this looks good. Will merge in a few unless I find anything!

Thanks!

chitoku-k · 2019-02-26T02:52:58Z

I appreciate all your reviews! Thanks.

…12647) ## Description Fixes a bug in `cloneTreeUntil` function in `gatsby-transformer-remark`, where it only returns the last child node. This function is expected to clone a `root` node, but the current implementation forgets to revert the variable back to the `root` node. Reassigning `clonedRoot = newNode` (originally this is the argument `root`) should fix this. By resolving this bug, it appears to correctly returns the entire root node of AST in `excerptAst` of the markdown node, instead of returning the partial result. ## Related Issues Related to #11237

chitoku-k changed the title ~~chore(gatsby-transformer-remark): add excerptAst to be exported as a GraphQL field~~ Jan 23, 2019

chitoku-k force-pushed the feature/transformer-remark branch from f1c15f4 to 2aa0c3a Compare January 23, 2019 18:36

chitoku-k force-pushed the feature/transformer-remark branch from de6c49b to 1295a30 Compare January 29, 2019 13:16

chitoku-k requested a review from pieh January 29, 2019 13:29

DSchau reviewed Jan 29, 2019

View reviewed changes

DSchau changed the title ~~fix(gatsby-transformer-remark): add excerptAst to be exported as a GraphQL field~~ Feb 21, 2019

chitoku-k added 7 commits February 23, 2019 18:02

fix(gatsby-transformer-remark): add excerptAst to be exported as a Gr…

6638b26

…aphQL field

fix(gatsby-transformer-remark): excerptAstCache was not saved

d9638f8

fix(gatsby-transformer-remark): await before creating excerpt

99bcea0

fix(e2e-tests): ensure remarkPlugin processes all ASTs

11c9fb5

fix(gatsby-transformer-remark): pass excerpt_separator to gray-matter…

f00d56e

… as well partially reverting gatsbyjs#11363

fix(gatsby-transformer-remark): remove excerptAstCacheKey

4ae0ac0

fix(gatsby-transformer-remark): remove cache mechanism for excerpts

c29290f

chitoku-k force-pushed the feature/transformer-remark branch from a5b45fd to c29290f Compare February 23, 2019 09:03

DSchau removed the status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response label Feb 25, 2019

DSchau approved these changes Feb 25, 2019

View reviewed changes

DSchau merged commit e59d4ca into gatsbyjs:master Feb 25, 2019

chitoku-k deleted the feature/transformer-remark branch February 26, 2019 02:53

chitoku-k mentioned this pull request Mar 18, 2019

fix(gatsby-transformer-remark): always include the root node of AST #12647

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(gatsby-transformer-remark): add excerptAst to be exported as a GraphQL field #11237

feat(gatsby-transformer-remark): add excerptAst to be exported as a GraphQL field #11237

chitoku-k commented Jan 23, 2019

chitoku-k commented Jan 24, 2019

chitoku-k commented Jan 24, 2019

DSchau commented Jan 24, 2019

chitoku-k commented Jan 24, 2019

DSchau left a comment

DSchau Jan 24, 2019

chitoku-k Jan 29, 2019

chitoku-k commented Feb 21, 2019

DSchau commented Feb 21, 2019

DSchau commented Feb 21, 2019

chitoku-k commented Feb 23, 2019

chitoku-k commented Feb 23, 2019

DSchau commented Feb 25, 2019

DSchau left a comment

chitoku-k commented Feb 26, 2019

feat(gatsby-transformer-remark): add excerptAst to be exported as a GraphQL field #11237

feat(gatsby-transformer-remark): add excerptAst to be exported as a GraphQL field #11237

Conversation

chitoku-k commented Jan 23, 2019

Description

chitoku-k commented Jan 24, 2019

chitoku-k commented Jan 24, 2019

DSchau commented Jan 24, 2019

chitoku-k commented Jan 24, 2019

DSchau left a comment

Choose a reason for hiding this comment

DSchau Jan 24, 2019

Choose a reason for hiding this comment

chitoku-k Jan 29, 2019

Choose a reason for hiding this comment

chitoku-k commented Feb 21, 2019

DSchau commented Feb 21, 2019

DSchau commented Feb 21, 2019

chitoku-k commented Feb 23, 2019

chitoku-k commented Feb 23, 2019

DSchau commented Feb 25, 2019

DSchau left a comment

Choose a reason for hiding this comment

chitoku-k commented Feb 26, 2019