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(blog): apply trailing slash to blog feed #9920

Merged
merged 4 commits into from
Mar 14, 2024
Merged

Conversation

OzakIOne
Copy link
Contributor

@OzakIOne OzakIOne commented Mar 6, 2024

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

tools like Algolia that use the feed files may behave unusual if trailing slash is enabled and not applied to links

Test Plan

feed.test.ts => has feed item for each post trailing slash

Deploy preview ??

Test links

rss.xml

Related issues/PRs

Fix #9829

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Mar 6, 2024
Copy link

netlify bot commented Mar 6, 2024

[V2]

Name Link
🔨 Latest commit 3fff0a5
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/65f30d16a306a200089531a5
😎 Deploy Preview https://deploy-preview-9920--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 configuration.

Copy link

github-actions bot commented Mar 6, 2024

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟠 66 🟢 98 🟢 96 🟢 100 🟠 88 Report
/docs/installation 🟠 64 🟢 96 🟢 100 🟢 100 🟠 88 Report
/docs/category/getting-started 🟠 75 🟢 100 🟢 100 🟢 90 🟠 88 Report
/blog 🟠 68 🟢 100 🟢 100 🟢 90 🟠 88 Report
/blog/preparing-your-site-for-docusaurus-v3 🟠 66 🟢 96 🟢 100 🟢 100 🟠 88 Report
/blog/tags/release 🟠 69 🟢 100 🟢 100 🟠 80 🟠 88 Report
/blog/tags 🟠 73 🟢 100 🟢 100 🟢 90 🟠 88 Report

Copy link

github-actions bot commented Mar 6, 2024

Size Change: 0 B

Total Size: 992 kB

ℹ️ View Unchanged
Filename Size
website/.docusaurus/globalData.json 75.4 kB
website/build/assets/css/styles.********.css 114 kB
website/build/assets/js/main.********.js 765 kB
website/build/index.html 37.9 kB

compressed-size-action

@OzakIOne
Copy link
Contributor Author

OzakIOne commented Mar 6, 2024

Found a todo

// TODO deduplicate: also present in @docusaurus/utils
function addTrailingSlash(str: string): string {
return str.endsWith('/') ? str : `${str}/`;
}

I think we can do it in this PR however should we keep the one in utils-common or utils ?

@slorber
Copy link
Collaborator

slorber commented Mar 7, 2024

  • utils is Node.js only utils
  • utils-common is Browser + Node.js

This code is env agnostic so it should rather be in utils-common, but is not because this shared package was introduced later. I'd prefer if you did this refactor in another PR because this will impact various files unrelated to this feature.

@slorber slorber added pr: bug fix This PR fixes a bug in a past release. to backport This PR is planned to be backported to a stable version of Docusaurus labels Mar 7, 2024
@OzakIOne OzakIOne marked this pull request as ready for review March 14, 2024 14:22
@OzakIOne OzakIOne requested a review from Josh-Cena as a code owner March 14, 2024 14:22
@slorber slorber changed the title feat(feed): apply trailing slash feat(blog): apply trailing slash to blog feed Mar 14, 2024
@slorber slorber changed the title feat(blog): apply trailing slash to blog feed fix(blog): apply trailing slash to blog feed Mar 14, 2024
@slorber slorber merged commit 14bec09 into main Mar 14, 2024
31 checks passed
@slorber slorber deleted the ozaki/feedTrailingSlash branch March 14, 2024 15:45
@ilg-ul
Copy link
Contributor

ilg-ul commented Mar 14, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release. to backport This PR is planned to be backported to a stable version of Docusaurus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a trailing slash to feed URLs if trailingSlash is enabled
4 participants