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(theme): canonical url should be not change after hydration if url accessed with/without trailing slash #9130

Merged

Conversation

ori-shalom
Copy link
Contributor

@ori-shalom ori-shalom commented Jul 7, 2023

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.
    I'm not really sure how to test it but we have it on every page of the site so I guess we can say there is a dogfooding...
  • 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

See #9128

Test Plan

Visit a page on the website with a trailing slash in the URL and inspect the canonical link using dev tools.
Then navigate to the same page without a trailing slash and inspect the canonical link using dev tools again.

If both versions of the page have the same URL then the bug is fixed.

Screen.Recording.2023-07-08.at.0.22.43.mov

Test links

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

Related issues/PRs

Closes #9128

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jul 7, 2023
@netlify
Copy link

netlify bot commented Jul 7, 2023

[V2]

Name Link
🔨 Latest commit 6474b3a
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/64ba985031040a0008fbbaf3
😎 Deploy Preview https://deploy-preview-9130--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.

@github-actions
Copy link

github-actions bot commented Jul 7, 2023

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟠 73 🟢 97 🟢 92 🟢 100 🟠 89 Report
/docs/installation 🟠 71 🟢 100 🟢 92 🟢 100 🟠 89 Report

ori-shalom and others added 2 commits July 17, 2023 13:38
@slorber
Copy link
Collaborator

slorber commented Jul 21, 2023

LGTM thanks 👍

Did some minor changes considering we have a applyTrailingSlash util method, and also normalized the hreflang URLs.

This PR improves things a bit and is good enough to merge.

Note I think we still have an SEO problem when aliasing docs, see #9170 (unfortunately not so easy to fix).

@slorber slorber changed the title Fix canonical url fix(theme): canonical url should be not change after hydration if url accessed with/without trailing slash Jul 21, 2023
@slorber slorber added the pr: bug fix This PR fixes a bug in a past release. label Jul 21, 2023
@slorber slorber merged commit 4ea0a70 into facebook:main Jul 21, 2023
30 of 31 checks passed
@slorber slorber added the to backport This PR is planned to be backported to a stable version of Docusaurus label Jul 21, 2023
@argos-ci
Copy link

argos-ci bot commented Jul 21, 2023

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) 🧿 Changes detected (Review) 91 changes Sep 21, 2023, 8:50 AM

slorber added a commit to VinceCYLiao/docusaurus that referenced this pull request Aug 3, 2023
… accessed with/without trailing slash (facebook#9130)

Co-authored-by: Joshua Chen <sidachen2003@gmail.com>
Co-authored-by: sebastienlorber <lorber.sebastien@gmail.com>
slorber added a commit that referenced this pull request Sep 20, 2023
… accessed with/without trailing slash (#9130)

Co-authored-by: Joshua Chen <sidachen2003@gmail.com>
Co-authored-by: sebastienlorber <lorber.sebastien@gmail.com>
slorber added a commit that referenced this pull request Sep 20, 2023
…lease (#9324)

Co-authored-by: Joshua Chen <sidachen2003@gmail.com>
Co-authored-by: sebastienlorber <lorber.sebastien@gmail.com>
Co-authored-by: Sébastien Lorber <slorber@users.noreply.github.com>
Co-authored-by: Ori Shalom <ori-shalom@users.noreply.github.com>
Co-authored-by: Mikey O'Toole <mikey@homotechsual.dev>
Co-authored-by: TheCatLady <52870424+TheCatLady@users.noreply.github.com>
fix flaky screenshots, add html data-has-hydrated attribute (#9256)
fix(theme-common): ThemedComponent should display something when JS is disabled (#9243)
fix(theme): canonical url should be not change after hydration if url accessed with/without trailing slash (#9130)
fix(theme): only set classname on ul elements if they have an existing class (#9099)
fix(content-docs): sidebar generator should return customProps for doc items (#9107)
@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 Sep 21, 2023
sahandseifi added a commit to notificationapi-com/docs that referenced this pull request Mar 10, 2024
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.

duplicate canonical URLs for paths with and without trailing slashes
4 participants