-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
feat: support content on RSS Feeds #5144
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,13 @@ | ||
// @TODO: This is a temporary hack until we migrate to the `nodejs/nodejs.dev` codebase | ||
import { writeFile } from 'fs/promises'; | ||
import { writeFile, readFile} from 'fs/promises'; | ||
import { join } from 'path'; | ||
import { Feed } from 'feed'; | ||
|
||
import { evaluate } from '@mdx-js/mdx' | ||
import { createElement } from 'react' | ||
import { renderToString } from 'react-dom/server' | ||
import remarkGfm from 'remark-gfm' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need remark and remark-gfm here? I thought mdx would already include remark built-ins. |
||
import remarkFrontmatter from 'remark-frontmatter' | ||
import * as runtime from 'react/jsx-runtime' | ||
import { getRelativePath } from './_helpers.mjs'; | ||
|
||
// imports the global site config | ||
|
@@ -29,6 +34,25 @@ export const generateBlogYearPages = cachedBlogData => | |
.then(data => [...new Set(data)]) | ||
.then(data => data.forEach(createBlogYearFile)); | ||
|
||
/** | ||
* Render markdown to html via mdx | ||
* @param {string} body | ||
* @returns {Promise<string>} | ||
* @see {https://github.com/mdx-js/mdx/discussions/1985} | ||
*/ | ||
export const renderMarkdown = async body => { | ||
const { default: mdx } = await evaluate(body, { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, afaik we already should have a rendered version of this at the time, no? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where exists renderred version? {
post: {
title: 'Node v19.4.0 (Current)',
author: 'Rafael Gonzaga',
date: 2023-01-06T13:16:26.671Z,
category: 'release',
slug: '/blog/release/v19.4.0',
file: 'v19.4.0.md'
}
}
{
post: {
title: 'Node v18.13.0 (LTS)',
author: 'Danielle Adams',
date: 2023-01-06T01:01:33.599Z,
category: 'release',
slug: '/blog/release/v18.13.0',
file: 'v18.13.0.md'
}
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is just what we're passing as a prop, you should check the parent functions, as I think I manually build this "post" object. The actual source of the files are somewhere in the parent-callers. |
||
...runtime, | ||
format: "md", | ||
development: false, | ||
// format: 'md' — treat file as plain vanilla markdown | ||
// Need to add the following remark plugins to support GFM and frontmatter | ||
// https://github.com/remarkjs/remark-gfm | ||
// https://github.com/remarkjs/remark-frontmatter | ||
remarkPlugins: [remarkGfm, remarkFrontmatter], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the least, I need remarkFrontmatter. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need github flavoured markdown afaik. (Not sure if we use it on our regular build process? And if we do, then yes we need it). |
||
}) | ||
return renderToString(createElement(mdx)) | ||
} | ||
Comment on lines
+37
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I referred to mdx-js/mdx#1985, but there may be a better way. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To covert all the ".md" files into HTML pages is a good choice, because an HTML page is easy to read other than the converted js files. +1 for you. However there're many complicated things to cope with if you do that. E.g: 'mdx' has already wrapped some functions of render packages. So do you mean 'mdx' cannot satisfy what you need so you want to remove this reference and change it to 'remark' directly? Nothing worried but just for a confirmation :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a blank line between this function and the next one? |
||
export const generateWebsiteFeeds = cachedBlogData => | ||
siteConfig.rssFeeds.forEach(metadata => { | ||
const feed = new Feed({ | ||
|
@@ -43,16 +67,21 @@ export const generateWebsiteFeeds = cachedBlogData => | |
? `/en/blog/${metadata.blogCategory}` | ||
: '/en/blog'; | ||
|
||
const mapBlogPostToFeed = post => | ||
feed.addItem({ | ||
const mapBlogPostToFeed = async post => { | ||
const markdownContent = await readFile(join(blogPath, post.category, post.file), 'utf8'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're over-engineering things here. The whole already rendered blog post should be provided directly from here https://github.com/nodejs/nodejs.org/blob/main/next.data.mjs#L16. Nextra already gives us the whole tree containing the full grey-matter, frontmatter and markdown content + parsed markdown content, afaik. If it doesn't provide the parsed markdown into HTML, then at least we don't need to re-read the file with Also we don't need to use |
||
const htmlContent = await renderMarkdown(markdownContent) | ||
return { | ||
title: post.title, | ||
content: htmlContent, | ||
id: `https://nodejs.org/en${post.slug}`, | ||
link: `https://nodejs.org/en${post.slug}`, | ||
author: post.author, | ||
date: new Date(post.date), | ||
}); | ||
}; | ||
}; | ||
|
||
cachedBlogData(blogCategoryOrAll) | ||
.then(({ blogData }) => blogData.posts.forEach(mapBlogPostToFeed)) | ||
.then(({ blogData }) => Promise.all(blogData.posts.map(mapBlogPostToFeed))) | ||
.then((feedItems) => feedItems.forEach(feedItem => feed.addItem(feedItem))) | ||
.then(() => writeFile(join(publicFeedPath, metadata.file), feed.rss2())); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you need to run
npm run format
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not add a prettier check GitHub ci action rather than catch format inconsistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do have one. I'm pretty sure the CI failed for that.