-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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(content-blog): links in feed should be absolute #9151
Changes from 2 commits
b607956
a339fc9
e65995e
aaa869a
af9cbe2
10ba5cb
6a8345d
84493db
f382af6
08e17db
df68905
646d22a
d34fda6
0016a18
2158d7c
a62b99f
c04e223
0afae8b
1f8dfad
da0fb44
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.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,12 +8,18 @@ | |
import {jest} from '@jest/globals'; | ||
import path from 'path'; | ||
import fs from 'fs-extra'; | ||
import {load as cheerioLoad} from 'cheerio'; | ||
import {readOutputHTMLFile} from '@docusaurus/utils'; | ||
import {blogPostContainerID} from '@docusaurus/utils-common'; | ||
import {DEFAULT_OPTIONS} from '../options'; | ||
import {generateBlogPosts} from '../blogUtils'; | ||
import {createBlogFeedFiles} from '../feed'; | ||
import type {LoadContext, I18n} from '@docusaurus/types'; | ||
import { | ||
createBlogFeedFiles, | ||
defaultCreateFeedItems as feedDefaultCreateFeedItems, | ||
} from '../feed'; | ||
import type {LoadContext, I18n, DocusaurusConfig} from '@docusaurus/types'; | ||
import type {BlogContentPaths} from '../types'; | ||
import type {PluginOptions} from '@docusaurus/plugin-content-blog'; | ||
import type {BlogPost, PluginOptions} from '@docusaurus/plugin-content-blog'; | ||
|
||
const DefaultI18N: I18n = { | ||
currentLocale: 'en', | ||
|
@@ -62,6 +68,32 @@ | |
}); | ||
} | ||
|
||
function isFullAbsolutePath(str: string) { | ||
const domain = 'https://domain.com'; | ||
const {origin} = new URL(str, domain); | ||
return origin !== domain; | ||
} | ||
|
||
async function generateLinksOfBlogPosts(outDir: string, blogPosts: BlogPost[]) { | ||
const linksOfBlogPosts: {[postId: string]: string[]} = {}; | ||
const pathOfFile = path.join(outDir, 'blog'); | ||
const promises = blogPosts.map(async (post) => { | ||
try { | ||
const content = await readOutputHTMLFile(post.id, pathOfFile, true); | ||
const $ = cheerioLoad(content); | ||
const anchorElements = $(`div#${blogPostContainerID} a`); | ||
if (anchorElements.length > 0) { | ||
const href = anchorElements.map((_, elm) => elm.attribs.href).toArray(); | ||
linksOfBlogPosts[post.id] = href; | ||
} | ||
} catch { | ||
// post is a draft | ||
} | ||
}); | ||
await Promise.all(promises); | ||
return linksOfBlogPosts; | ||
} | ||
|
||
describe.each(['atom', 'rss', 'json'])('%s', (feedType) => { | ||
const fsMock = jest.spyOn(fs, 'outputFile').mockImplementation(() => {}); | ||
|
||
|
@@ -140,7 +172,7 @@ | |
|
||
expect( | ||
fsMock.mock.calls.map((call) => call[1] as string), | ||
).toMatchSnapshot(); | ||
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (18)atom › has feed item for each post
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (18)rss › has feed item for each post
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (18)json › has feed item for each post
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (16)atom › has feed item for each post
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (16)rss › has feed item for each post
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (16)json › has feed item for each post
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (16.14)atom › has feed item for each post
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (16.14)rss › has feed item for each post
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (16.14)json › has feed item for each post
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Windows Tests (18)atom › has feed item for each post
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Windows Tests (18)rss › has feed item for each post
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Windows Tests (18)json › has feed item for each post
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Windows Tests (16)atom › has feed item for each post
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Windows Tests (16)rss › has feed item for each post
Check failure on line 175 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Windows Tests (16)json › has feed item for each post
|
||
fsMock.mockClear(); | ||
}); | ||
|
||
|
@@ -192,7 +224,99 @@ | |
|
||
expect( | ||
fsMock.mock.calls.map((call) => call[1] as string), | ||
).toMatchSnapshot(); | ||
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (18)atom › filters to the first two entries
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (18)rss › filters to the first two entries
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (18)json › filters to the first two entries
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (16)atom › filters to the first two entries
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (16)rss › filters to the first two entries
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (16)json › filters to the first two entries
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (16.14)atom › filters to the first two entries
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (16.14)rss › filters to the first two entries
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (16.14)json › filters to the first two entries
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Windows Tests (18)atom › filters to the first two entries
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Windows Tests (18)rss › filters to the first two entries
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Windows Tests (18)json › filters to the first two entries
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Windows Tests (16)atom › filters to the first two entries
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Windows Tests (16)rss › filters to the first two entries
Check failure on line 227 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Windows Tests (16)json › filters to the first two entries
|
||
fsMock.mockClear(); | ||
}); | ||
}); | ||
|
||
describe('Test defaultCreateFeedItems', () => { | ||
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. That test looks super complex to me and I don't understand why. Just call 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 that I don't need to add new test case; instead I just need to update the snapshot. Am I correct ? |
||
const fsMock = jest.spyOn(fs, 'outputFile').mockImplementation(() => {}); | ||
it('links in feeds are resolved correctly', async () => { | ||
const siteDir = path.join(__dirname, '__fixtures__', 'website'); | ||
const outDir = path.join(siteDir, 'build-snap'); | ||
const siteConfig = { | ||
title: 'Hello', | ||
baseUrl: '/myBaseUrl/', | ||
url: 'https://docusaurus.io', | ||
favicon: 'image/favicon.ico', | ||
} as DocusaurusConfig; | ||
|
||
const context = { | ||
siteDir, | ||
siteConfig, | ||
i18n: DefaultI18N, | ||
outDir, | ||
} as LoadContext; | ||
|
||
const options = { | ||
path: 'blog', | ||
routeBasePath: 'blog', | ||
tagsBasePath: 'tags', | ||
authorsMapPath: 'authors.yml', | ||
include: DEFAULT_OPTIONS.include, | ||
exclude: DEFAULT_OPTIONS.exclude, | ||
feedOptions: { | ||
type: ['atom', 'rss', 'json'], | ||
copyright: 'Copyright', | ||
}, | ||
readingTime: ({content, defaultReadingTime}) => | ||
defaultReadingTime({content}), | ||
truncateMarker: /<!--\s*truncate\s*-->/, | ||
} as PluginOptions; | ||
|
||
const blogPosts = await generateBlogPosts( | ||
getBlogContentPaths(siteDir), | ||
context, | ||
options, | ||
); | ||
|
||
await createBlogFeedFiles({ | ||
blogPosts, | ||
options, | ||
siteConfig: context.siteConfig, | ||
outDir: context.outDir, | ||
locale: 'en', | ||
}); | ||
|
||
const originalLinksInBlogs: {[id: string]: Array<string>} = | ||
await generateLinksOfBlogPosts(outDir, blogPosts); | ||
|
||
const blogPostsWithLinks = blogPosts.filter( | ||
(post) => originalLinksInBlogs[post.id], | ||
); | ||
|
||
const feedsWithLinks = await feedDefaultCreateFeedItems({ | ||
blogPosts: blogPostsWithLinks, | ||
siteConfig, | ||
outDir, | ||
}); | ||
|
||
feedsWithLinks.forEach((feed) => { | ||
const $ = cheerioLoad(feed.content ?? ''); | ||
const linksInFeed = $('a') | ||
.map((_, elm) => elm.attribs.href) | ||
.toArray(); | ||
const idOfBlogPost = feed.id!.replace( | ||
new URL(`${siteConfig.baseUrl}blog`, siteConfig.url).href, | ||
'', | ||
); | ||
const originalLinksInBlog = originalLinksInBlogs[idOfBlogPost]; | ||
const {permalink = ''} = | ||
blogPostsWithLinks.find((post) => post.id === idOfBlogPost)?.metadata || | ||
{}; | ||
|
||
originalLinksInBlog!.forEach((originalLinkInBlog, idx) => { | ||
const linkToTest = isFullAbsolutePath(originalLinkInBlog) | ||
? originalLinkInBlog | ||
: new URL(originalLinkInBlog, new URL(permalink, siteConfig.url)) | ||
.href; | ||
|
||
expect(linkToTest).toEqual(linksInFeed[idx]); | ||
}); | ||
}); | ||
expect( | ||
fsMock.mock.calls.map((call) => call[1] as string), | ||
).toMatchSnapshot(); | ||
Check failure on line 319 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (18)Test defaultCreateFeedItems › links in feeds are resolved correctly
Check failure on line 319 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (16)Test defaultCreateFeedItems › links in feeds are resolved correctly
Check failure on line 319 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Tests (16.14)Test defaultCreateFeedItems › links in feeds are resolved correctly
Check failure on line 319 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Windows Tests (18)Test defaultCreateFeedItems › links in feeds are resolved correctly
Check failure on line 319 in packages/docusaurus-plugin-content-blog/src/__tests__/feed.test.ts GitHub Actions / Windows Tests (16)Test defaultCreateFeedItems › links in feeds are resolved correctly
|
||
fsMock.mockClear(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,7 +70,7 @@ async function generateBlogFeed({ | |
return feed; | ||
} | ||
|
||
async function defaultCreateFeedItems({ | ||
export async function defaultCreateFeedItems({ | ||
blogPosts, | ||
siteConfig, | ||
outDir, | ||
|
@@ -106,6 +106,14 @@ async function defaultCreateFeedItems({ | |
const $ = cheerioLoad(content); | ||
|
||
const link = normalizeUrl([siteUrl, permalink]); | ||
|
||
$(`div#${blogPostContainerID} a`).each((_, elm) => { | ||
const {href} = elm.attribs; | ||
if (href) { | ||
elm.attribs.href = String(new URL(href, link)); | ||
} | ||
}); | ||
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. LGTM 👍 We will also need to convert image links to absolute, see https://validator.w3.org/feed/docs/warning/ContainsRelRef.html 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. Done. Image links now are absolutized. |
||
|
||
const feedItem: BlogFeedItem = { | ||
title: metadataTitle, | ||
id: link, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
--- | ||
title: 'Test if href in feed resolved correctly' | ||
--- | ||
|
||
[absolute full url](https://github.com/facebook/docusaurus) | ||
|
||
[absolute url with implicit domain name](/tests/blog/2023/07/19/b) | ||
|
||
[relative url](2023-07-19-b.mdx) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
# Test Relative Path |
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 don't think we need that complexity inr our tests
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.
The test case is removed