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 images in the RSS feed #1245

Merged
merged 12 commits into from
May 6, 2020
Merged
30 changes: 16 additions & 14 deletions gatsby-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require('./config/prismjs/usage')

const apiMiddleware = require('./src/server/middleware/api')
const redirectsMiddleware = require('./src/server/middleware/redirects')
const makeFeedHtml = require('./plugins/utils/makeFeedHtml')
const { BLOG } = require('./src/consts')

const title = 'Data Version Control · DVC'
Expand Down Expand Up @@ -126,27 +127,28 @@ const plugins = [
description,
output: '/blog/rss.xml',
query: `
{
allBlogPost(
sort: { fields: [date], order: DESC }
) {
nodes {
html
slug
title
date
description
}
{
allBlogPost(
sort: { fields: [date], order: DESC }
) {
nodes {
htmlAst
slug
title
date
description
}
}
`,
}
`,
serialize: ({ query: { site, allBlogPost } }) => {
return allBlogPost.nodes.map(node => {
const html = makeFeedHtml(node.htmlAst, site.siteMetadata.siteUrl)
return Object.assign(
{},
{
/* eslint-disable-next-line @typescript-eslint/camelcase */
custom_elements: [{ 'content:encoded': node.html }],
custom_elements: [{ 'content:encoded': html }],
title: node.title,
date: node.date,
description: node.description,
Expand All @@ -170,7 +172,7 @@ const plugins = [
}
}
}
`
`
},
resolve: `gatsby-plugin-feed`
rogermparent marked this conversation as resolved.
Show resolved Hide resolved
},
Expand Down
53 changes: 53 additions & 0 deletions plugins/utils/makeFeedHtml.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
const { imageWrapperClass } = require('gatsby-remark-images/constants')
const unified = require('unified')
const { convertHastToHtml } = require('./convertHast.js')
const { select, selectAll } = require('hast-util-select')

const rootToAbsolute = siteUrl => tree => {
selectAll('a', tree).forEach(node => {
Copy link
Member

Choose a reason for hiding this comment

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

let's put a link here gatsbyjs/gatsby#14133 to remove this when it's fixed upstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me!

if (node.properties.href.startsWith('/')) {
node.properties.href = siteUrl + node.properties.href
}
})
selectAll('img', tree).forEach(node => {
if (node.properties.src.startsWith('/')) {
node.properties.src = siteUrl + node.properties.src
}
})
return tree
}

const unwrapImages = () => tree => {
selectAll(`.${imageWrapperClass}`, tree).forEach(node => {
// Set the fallback image as the wrapper's only child, and then
// give that image the wrapper's original style.
const fallbackImg = select('img', node)
node.children = [
{
...fallbackImg,
properties: {
...fallbackImg.properties,
style: 'max-width: 100%; margin: auto;'
Copy link
Member

Choose a reason for hiding this comment

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

qq- does it makes/do we respect our custom image sizing logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This just changes the wrapper's internal image to have max-width: 100% so larger images don't extend offscreen on narrower displays and margin: auto so smaller ones are centered.
The original max-width on the wrapper that the existing plugin operates on is untouched, as well as any other properties on the wrapper.

}
}
]
})
return tree
}

function makeFeedHtml(htmlAst, siteUrl) {
Copy link
Member

Choose a reason for hiding this comment

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

really annoying that gastby plugin feed does not handle this ... kinda feels like all of this is a requirement for that plugin to be useful

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree to a certain extent, especially for our use case, but RSS feeds are very often used for something like podcasts where the encoded content is more of a summary than the whole blog (ads are a lot harder in an RSS reader, after all), and the encoded content is kept simple.

This isn't to say the default and suggested queries of gatsby-plugin-feed are good, I think naively passing through the content's raw html string is kind of poor way to go about things, but it's not as if the plugin is useless without this kind of content reprocessing.

// We add the rootToAbsolute processor before usage because it depends on siteUrl.
return convertHastToHtml(
unified()
/*
All images processed by Gatsby to be responsive are "unwrapped" into
their fallback 'img' nodes, as RSS doesn't work with the tricks that
true HTML does.
*/
.use(unwrapImages)
.use(rootToAbsolute, siteUrl)
.runSync(htmlAst)
)
}

module.exports = makeFeedHtml