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(content-blog): make RSS feed generation work with slugs with .html extension #8158

Merged
merged 2 commits into from Oct 6, 2022

Conversation

Pranav2612000
Copy link
Contributor

@Pranav2612000 Pranav2612000 commented Oct 1, 2022

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

Before this commit, adding slugs ending in .html to the frontmatter blogs resulted in the build process breaking. This PR fixes this issue

Test Plan

Added Tests for the emitUtils.readOutputHTMLFile function in which the changes were to be made. Sample file also has been added in website/_dogfooding

Test links

Deploy preview: https://deploy-preview-8158--docusaurus-2.netlify.app/

Related issues/PRs

Closes #8139

@Pranav2612000
Copy link
Contributor Author

@slorber I've created a PR. Can you take a look. I've tried to cover all these points you've mentioned

  • Reproduce by adding slug: /x/y/z.html to any of our website blog post
  • Add this to website/_dogfooding/_blog tests (visible here: https://docusaurus.io/tests/blog) => we'll keep this in git and ensure this issue doesn't come back
  • Fix the bug in readOutputHTMLFile() + cover in unit test
  • Ensure it works fine under all trailingSlash combinations (CI should tell you, we build with trailingSlash: isDeployPreview,)

@netlify
Copy link

netlify bot commented Oct 1, 2022

[V2]

Name Link
🔨 Latest commit 260c6a3
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/633edc93d87cf400093ad4d3
😎 Deploy Preview https://deploy-preview-8158--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

github-actions bot commented Oct 1, 2022

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟠 63 🟢 98 🟢 100 🟢 100 🟠 80 Report
/docs/installation 🟠 79 🟢 100 🟢 100 🟢 100 🟢 90 Report

@Josh-Cena Josh-Cena added the pr: bug fix This PR fixes a bug in a past release. label Oct 2, 2022
@Josh-Cena Josh-Cena changed the title fix: prevent yarn build from breaking when html slug is used fix(content-blog): make RSS feed generation work with slugs with .html extension Oct 2, 2022
Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM 👍

Just fixed a edge case where the slug could contain .html in other places than the end

@slorber slorber merged commit efbd0cb into facebook:main Oct 6, 2022
@slorber slorber added the to backport This PR is planned to be backported to a stable version of Docusaurus label Oct 6, 2022
slorber added a commit that referenced this pull request Oct 28, 2022
…l extension (#8158)

Co-authored-by: sebastienlorber <lorber.sebastien@gmail.com>
@slorber slorber added backported This PR has been backported to a stable version of Docusaurus and removed to backport This PR is planned to be backported to a stable version of Docusaurus labels Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported This PR has been backported to a stable version of Docusaurus CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slug with explicit .html extension breaks build
4 participants