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-source-contentful): LongText fields require markdown transformer #20398

Merged
merged 9 commits into from
Jan 15, 2020

Conversation

slk333
Copy link
Contributor

@slk333 slk333 commented Jan 3, 2020

TLDR: The main problem is that if you follow the current documentation, you end up with raw markdown in your pages instead of the expected formatted content

As shown here: #3205 (comment)

The source plugins still exposes the raw markdown but generally you'd want to use the transformed HTML as in the above code sample.

this means doing the following will return raw markdown instead of html.

body{
  body
  }
}

You have to use a transformer plugin to transform the markdown into html.
The doc was misleading since it suggested that using LongText can be used without a transformer, and said that the markdown was transformed by Gatsby (incorrect). As a result, if someone were to follow the example, he would get raw markdown displaying in the webpage, as opposed to the expected result.

Description

Documentation

Related Issues

…by-transformer-remark

As shown here: #3205 (comment)
you cannot simply do 
```{
body{
 body}
}
since the body will be raw markdown, which is not html
@slk333 slk333 requested a review from a team as a code owner January 3, 2020 14:34
@vladar vladar changed the title fix[gatsby-source-contentful] LongText fields require markdown transformer fix(gatsby-source-contentful): LongText fields require markdown transformer Jan 9, 2020
slk333 and others added 4 commits January 9, 2020 20:31
Co-Authored-By: Vladimir Razuvaev <vladimir.razuvaev@gmail.com>
The text might not be perfect but it clearly explains what happens. Feel free to reuse it or change it.
@slk333
Copy link
Contributor Author

slk333 commented Jan 9, 2020

pinging @jserrao as he authored this part I'm trying to clarify.

@jserrao
Copy link
Contributor

jserrao commented Jan 9, 2020

@slk333 - I remember when I wrote this, you had to include body inside the query or it wouldn't work correctly - honestly can't remember the specifics of whether it returned markdown or HTML. I trust that your improvement is correct slk333.

@slk333
Copy link
Contributor Author

slk333 commented Jan 9, 2020

at the moment, here's what you get when you query the child node:
Screenshot 2020-01-09 at 22 40 54
you get raw markdown

As for React Markdown:

The advantage of a React Markdown component was that you don't have to do the inline html insert, which looks bad. You could simply take the markdown node and do:

<ReactMarkdown source={data.contentfulTest.body.body} />

credit @bowenac
but as a drawback you miss some features of the transformer.

@jserrao
Copy link
Contributor

jserrao commented Jan 10, 2020 via email

vladar
vladar previously approved these changes Jan 13, 2020
Copy link
Contributor

@vladar vladar left a comment

Choose a reason for hiding this comment

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

Content looks accurate to me (tried with contentful) but I think we need another pair of eyes from @gatsbyjs/learning to approve the text

@vladar vladar requested a review from a team January 13, 2020 16:45
@vladar vladar added the type: documentation An issue or pull request for improving or updating Gatsby's documentation label Jan 13, 2020
@slk333
Copy link
Contributor Author

slk333 commented Jan 13, 2020

sorry I didn't intend to delay the review, didn't see you asked for a review. Thanks @vladar

@vladar
Copy link
Contributor

vladar commented Jan 13, 2020

Looks like lint checks are failing. Try running yarn format on a repo when you are ready. Also, there is a WIP label in case if you still work on this PR.

P.S. Thanks for this fix by the way!

laurieontech
laurieontech previously approved these changes Jan 13, 2020
Copy link
Contributor

@laurieontech laurieontech left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for the clear explanation.

@slk333
Copy link
Contributor Author

slk333 commented Jan 14, 2020

Hi @vladar , thanks for your help! I'm still a bit confused on how to fix the lint check failure. Do I have to download the whole gatsby repo and run yarn format directly on that repo?
Or maybe I can open the document and click on "Format document" on VSCode with the prettier plug-in? When I tried to do that, it also formats other parts of the document, so I'm not sure if it's intended or not.

@laurieontech
Copy link
Contributor

Was able to run it for you :) Thanks so much for the PR!

@laurieontech laurieontech merged commit 1ad7bbb into master Jan 15, 2020
@delete-merged-branch delete-merged-branch bot deleted the slk333-patch-1 branch January 15, 2020 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation An issue or pull request for improving or updating Gatsby's documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants