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

Using trailingSlash: false on GH-pages causes the root page to flash "Not Found" #5077

Closed
5 tasks done
jsamr opened this issue Jun 28, 2021 · 20 comments · Fixed by #5082
Closed
5 tasks done

Using trailingSlash: false on GH-pages causes the root page to flash "Not Found" #5077

jsamr opened this issue Jun 28, 2021 · 20 comments · Fixed by #5082
Labels
bug An error in the Docusaurus core causing instability or issues with its execution

Comments

@jsamr
Copy link
Contributor

jsamr commented Jun 28, 2021

🐛 Bug Report

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • I have tried creating a repro with https://new.docusaurus.io
  • I have read the console error message carefully (if applicable)

Description

Using trailingSlash: false on GH-pages causes the root page to flash "Not Found"

Have you read the Contributing Guidelines on issues?

Yes.

Steps to reproduce

  1. Create a new project with npx @docusaurus/init@latest init trailing-bug classic
  2. Configure the project for GH-pages and set trailingSlash: false in docusaurus.config.js
  3. Deploy
  4. Visit the root, e.g. https://{organizationName}.github.io/{projectName}/

Expected behavior

The home page should be displayed without flashing.

Actual behavior

The home page should be displayed after flashing "Not Found" route.

Your environment

Reproducible demo

https://github.com/jsamr/docusaurus-trailing-slash

Below is a slowed-down screencast I recorded for the original project where I encountered this issue (react-native-render-html).

404.mp4

Additional Notes

Variants

Visiting https://{organizationName}.github.io/{projectName} instead of https://{organizationName}.github.io/{projectName}/ will cause a redirect to https://{organizationName}.github.io/{projectName}/ with the same flashing issue.

Workaround

I noticed that if I added a build step to copy docusaurus-trailing-slash.html into index.html, the flashing would not occur anymore. Below is an example of plugin to do just that:

const path = require('path');
const fs = require('fs/promises');

// quick fix for GH pages trailing slash in root issue
async function fixIndex({ outDir, baseUrl, siteConfig: { url, projectName } }) {
  // Don't proceed if baseUrl is not defined
  if (!baseUrl) {
    return;
  }

  const rootFile = path.join(outDir, projectName + '.html');

  // copy baseurl file to index
  if ((await fs.stat(rootFile)).isFile()) {
    await fs.copyFile(rootFile, path.join(outDir, 'index.html'));
  }
}

module.exports = function () {
  return {
    plugin: 'doc-docusaurus-ghpages-plugin',
    async postBuild(props) {
      await fixIndex(props);
    }
  };
};
@jsamr jsamr added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Jun 28, 2021
@slorber
Copy link
Collaborator

slorber commented Jun 29, 2021

Thanks, investigating this right now

This is not normal that index.html does not exist anymore here: https://github.com/jsamr/docusaurus-trailing-slash/tree/gh-pages

@slorber
Copy link
Collaborator

slorber commented Jun 29, 2021

I suspect it only affects trailingSlash: false + baseUrl (required when using GH pages without a custom domain)

@F43nd1r reported here that it works fine for GH pages: #5026 (comment)

However his site is using a custom domain so it's not using baseurl: https://www.acra.ch/

@slorber
Copy link
Collaborator

slorber commented Jun 29, 2021

Can confirm, we are also affected on localized sites: https://docusaurus.io/fr/

@slorber
Copy link
Collaborator

slorber commented Jun 29, 2021

Thanks, hopefully you reported this.

Should be fixed in master, please test with canary release https://docusaurus.io/community/canary

@jsamr
Copy link
Contributor Author

jsamr commented Jun 29, 2021

My pleasure! Just tested this (2.0.0-beta.759298296) and indeed the index.html file was generated. Unfortunately, the sitemap.xml file was missing:

> tree -L 1 apps/website/build
apps/website/build
├── api
├── assets
├── blog
├── docs
├── favicons
├── img
├── video
├── 404.html
├── api.html
├── blog.html
├── index.html
├── .nojekyll
├── opensearch.xml
└── search.html

Before (2.0.0-beta.4d93c894f):

> tree -L 1 apps/website/build
apps/website/build
├── api
├── assets
├── blog
├── docs
├── favicons
├── img
├── video
├── 404.html
├── api.html
├── blog.html
├── .nojekyll
├── opensearch.xml
├── react-native-render-html.html
├── search.html
└── sitemap.xml

@slorber
Copy link
Collaborator

slorber commented Jun 30, 2021

I am unable to build your real site unfortunately so can't really debug it.

However I used https://github.com/jsamr/docusaurus-trailing-slash:

Before:

drwxr-xr-x  12 sebastienlorber  staff    384 Jun 30 11:38 .
drwxr-xr-x  17 sebastienlorber  staff    544 Jun 30 11:38 ..
-rw-r--r--   1 sebastienlorber  staff      0 Jun 30 11:38 .nojekyll
-rw-r--r--   1 sebastienlorber  staff   7042 Jun 30 11:38 404.html
drwxr-xr-x   5 sebastienlorber  staff    160 Jun 30 11:38 assets
drwxr-xr-x   9 sebastienlorber  staff    288 Jun 30 11:38 blog
-rw-r--r--   1 sebastienlorber  staff  11816 Jun 30 11:38 blog.html
drwxr-xr-x   5 sebastienlorber  staff    160 Jun 30 11:38 docs
-rw-r--r--   1 sebastienlorber  staff  71168 Jun 30 11:38 docusaurus-trailing-slash.html
drwxr-xr-x   9 sebastienlorber  staff    288 Jun 30 11:38 img
-rw-r--r--   1 sebastienlorber  staff   7128 Jun 30 11:38 markdown-page.html
-rw-r--r--   1 sebastienlorber  staff   3316 Jun 30 11:38 sitemap.xml

After:

 ~/Desktop/docusaurus-trailing-slash  master ●  ll build                                                                                                                            ✓  10047  11:41:05
total 208
drwxr-xr-x  12 sebastienlorber  staff    384 Jun 30 11:41 .
drwxr-xr-x  18 sebastienlorber  staff    576 Jun 30 11:40 ..
-rw-r--r--   1 sebastienlorber  staff      0 Jun 30 11:40 .nojekyll
-rw-r--r--   1 sebastienlorber  staff   7044 Jun 30 11:40 404.html
drwxr-xr-x   5 sebastienlorber  staff    160 Jun 30 11:40 assets
drwxr-xr-x   9 sebastienlorber  staff    288 Jun 30 11:41 blog
-rw-r--r--   1 sebastienlorber  staff  11818 Jun 30 11:40 blog.html
drwxr-xr-x   5 sebastienlorber  staff    160 Jun 30 11:40 docs
drwxr-xr-x   9 sebastienlorber  staff    288 Jun 30 11:40 img
-rw-r--r--   1 sebastienlorber  staff  72357 Jun 30 11:40 index.html
-rw-r--r--   1 sebastienlorber  staff   7130 Jun 30 11:40 markdown-page.html
-rw-r--r--   1 sebastienlorber  staff   3317 Jun 30 11:41 sitemap.xml

Considering we didn't change any significant thing in the sitemap plugin, I find it surprising that it magically disappeared. Please double-check and see if you can create a smaller repro that I can actually run

@jsamr
Copy link
Contributor Author

jsamr commented Jun 30, 2021

@slorber
It has indeed nothing to do with the trailing slash fix (it seems)! But a breaking change in when the postBuild lifecycle method is called on plugins. As of 2.0.0-beta.759298296, postBuild is called while sitemap.xml is not yet present, however it was present in beta.2. Reproduction of this behavior can be found in the sitemap branch . Just run yarn build there and you'll see what I mean. If you wish, I can open a separate report.

EDIT The "good" behavior was still happening in version 2.0.0-beta.4d93c894f

@slorber
Copy link
Collaborator

slorber commented Jun 30, 2021

@jsamr the sitemap plugin is using the postBuild hook, so it is present after the postBuild hook has run.

If you try to read sitemap.xml inside another plugin' postBuild hook, it's an antipattern to avoid because all postBuild hooks of all plugins will run concurrently, leading to a non-deterministic behavior

Plugins are designed to be self-contained and run concurrently, not to interact with each others with some kind of ordering/priority logic

If you need to do something after the build globally, you'd rather just do yarn build && yarn myPostBuildScript

If you need to do this in a plugin, I think your usecase may belong to the "extend existing plugins" feature request (#4138): you may somehow want to extend the sitemap plugin to add additional logic on the postBuild hook, after the sitemap has been written to disk. Not sure what you are doing with that sitemap though so can't tell for sure.

@jsamr
Copy link
Contributor Author

jsamr commented Jun 30, 2021

@slorber I see! I didn't realize the sitemap.xml was generated in a postBuild hook. Well I used to alter the sitemap by adding a trailing slash to the root route, but that is now fixed with #5082

@Josh-Cena Josh-Cena removed the status: needs triage This issue has not been triaged by maintainers label Feb 22, 2022
@B4nan
Copy link
Contributor

B4nan commented Aug 18, 2022

Looks like this is still happening with 2.0.1 stable.

https://crawlee.dev/docs/quick-start/ vs https://crawlee.dev/docs/quick-start (GH renders them the same but first URL has a trailing slash)
https://github.com/apify/crawlee/blob/master/website/docusaurus.config.js#L39

The redirect happens on client side, the server side response is 404.

Should I open new issue with more details?

@Josh-Cena
Copy link
Collaborator

@B4nan You set trailingSlash: false, so pages with trailing slash would have a flash of 404. That seems working as intended to me, because GH pages does not normalize trailing slashes. This should not happen for normal URLs you obtained from the address bar.

@B4nan
Copy link
Contributor

B4nan commented Aug 18, 2022

Oh I see, the issue was about the root page specifically.

Can't we have a 301 redirect instead somehow? I guess it would require a real server instead of just GH pages with static content?

@Josh-Cena
Copy link
Collaborator

Correct, you can't do the normalization on the server side with GH pages. You may have luck with the client-redirects plugin.

@B4nan
Copy link
Contributor

B4nan commented Aug 18, 2022

As said, the redirect already happens just fine, but on client side (so with the same side effect as the OP - 404 flashing in the background).

@Josh-Cena
Copy link
Collaborator

You are not really using the client redirects to normalize trailing slashes. What's happening right now is that the server does not recognize this location and sends 404.html, but the router does, and hydrates it with the right content. What you want to do is to let the server respond with the right HTML instead of 404. This happens in the client-redirects plugin via emitting an extra HTML file, so that the location with trailing slash still corresponds to an HTML file, but that HTML file just contains JS that causes an instant client redirect.

@Josh-Cena
Copy link
Collaborator

Actually... I'm not even sure if that's possible with client-redirects. Maybe @slorber knows since he researched it a lot?

@B4nan
Copy link
Contributor

B4nan commented Aug 18, 2022

Oh I see, thanks for the explanation. Not sure if it would be worth doing, as we would want this behaviour for every single URL, meaning we would double the amount of URLs just for this simple use case (and we use versioning, so this would be again multiplied for each version).

@Josh-Cena
Copy link
Collaborator

Right, but we have a createRedirects function designed to handle this case—it can be batch-applied to every route and generate a "normalization route".

@slorber
Copy link
Collaborator

slorber commented Aug 18, 2022

If this annoys you, my only suggestion is to move out of Github pages and use a better host instead 😄

using trailingSlash true might give better results 🤷‍♂️

@B4nan
Copy link
Contributor

B4nan commented Aug 18, 2022

Actually the createRedirects works just fine, thanks for the hint. Feels much better to have 200 with a client side redirect and canonical link instead of 404 :]

apify/crawlee@1ba5587

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants