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

Enable clean / extension-less url #677

Merged
merged 11 commits into from
Jun 6, 2018
Merged

Conversation

endiliey
Copy link
Contributor

@endiliey endiliey commented May 19, 2018

Motivation

Closes #413

This is a feature request in issue #413, and #478 (comment)

Some of Docosaurus URL look very cryptic/un-clean. Instead of blog/a.html, blog/b.html, and blog/c.html, user should be able to choose if they prefer to see blog/a, blog/b and blog/c

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

  • New config cleanUrl in siteConfig.js,
    If true, all linking from each page will be without .html except custom pages that has hardcoded linking value.

Before:
before-link

After:
after-link

  • yarn start will allow extension-less url (server routing)
    Which means request to docs/a.html is the same as docs/a
    Test yarn start server routing on development server
cd website
yarn start

http://localhost:3000/docs/en/installation
dev-1

http://localhost:3000/docs/en/installation.html
dev-2

  • yarn build will generate extra file when cleanUrl at siteConfig.js is true

Test yarn build to see generated files

cd website
yarn build
tree -a -I "css|CNAME|.circleci|img|js" build/Docusaurus/ > build.txt

Results are expected
Full generated files structure can be seen at https://gist.github.com/endiliey/cd184c237bc1c0b544677ad19cc8ece7

build/Docusaurus/
├── about-slash
│   └── index.html
├── about-slash.html
├── blog
│   ├── 2017
│   │   └── 12
│   │       └── 14
│   │           ├── introducing-docusaurus
│   │           │   └── index.html
│   │           └── introducing-docusaurus.html

├── docs
│   ├── 1.0.11
│   │   ├── api-pages
│   │   │   └── index.html
│   │   ├── api-pages.html
cd website
yarn build
serve build/Docosaurus

serve

http://localhost:5000/docs/en/site-preparation
prod-1-doc

http://localhost:5000/docs/en/site-preparation.html
prod-with-html

http://localhost:5000/blog/2018/04/30/How-I-Converted-Profilo-To-Docusaurus
prod-blog-clean

http://localhost:5000/blog/2018/04/30/How-I-Converted-Profilo-To-Docusaurus.html
prod-blog-html

http://localhost:5000/en/about-slash
prod-custom-clean

http://localhost:5000/en/about-slash.html
prod-custom-html

All seems to be working fine. Every url is accessible with and without .html

Alternatively, you can test production build in Netlify
https://deploy-preview-677--docusaurus-preview.netlify.com/

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label May 19, 2018
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented May 19, 2018

Deploy preview for docusaurus-preview ready!

Built with commit 52376f0

https://deploy-preview-677--docusaurus-preview.netlify.com

@endiliey endiliey force-pushed the clean-url branch 4 times, most recently from 6abeda5 to b36f0ac Compare May 19, 2018 16:54
@endiliey endiliey changed the title [WIP] Enable clean / extension-less url Enable clean / extension-less url May 19, 2018
@endiliey endiliey force-pushed the clean-url branch 2 times, most recently from 004d141 to 63d4982 Compare May 20, 2018 15:04
@ericnakagawa
Copy link
Contributor

This is a neat feature. Thanks for supporting the old urls as well!

@yansern
Copy link

yansern commented May 22, 2018

Just tried this out myself while I was testing the other non-related issue! Great work!

@JoelMarcey
Copy link
Contributor

@endiliey Excellent! Looks like others have tested it too, which always makes me feel better about a bigger change like this 😄

Are you ready for this to go in? If so, I will update the branch and we can merge.

@yangshun Are you good?

@endiliey
Copy link
Contributor Author

I am not in a rush with this PR, i'm out of reach from my laptop until this weekend because i'm currently hospitalized. But i have tested it on dev & prod build in local web server before and there is no problem yet.

But note that custom pages (e.g: pages/en/index.js) with hardcoded link still link to docs/installation.html

https://github.com/facebook/Docusaurus/blob/a603aff193096195bd805b5755f662d981d0857e/website/pages/en/index.js#L51

Do we want to change it to extension-less one ?

@JoelMarcey
Copy link
Contributor

Hi @endiliey

I am not in a rush with this PR, i'm out of reach from my laptop until this weekend because i'm currently hospitalized.

Best wishes to you. 🙏

But note that custom pages (e.g: pages/en/index.js) with hardcoded link still link to docs/installation.html

We can change those in a separate PR, I think.

@endiliey
Copy link
Contributor Author

I am ready anytime. I'll leave it to your decision. Waiting for @yangshun approval is also a good idea 😉

@yangshun
Copy link
Contributor

I'll give it a thorough look tonight! One concern I have is that will the blog comments will show different contents depending on whether it's the index.html version or the clean URL version? Not sure if FB treats them as canonical URLs.

@endiliey
Copy link
Contributor Author

endiliey commented May 22, 2018

I just tested that and it is actually the same url. Regardless of .html or /index.html or /. The data-href from facebook comment is not the current url and always point to the clean url (if siteConfig cleanUrl is true)

I just added 'Wow, awesome' to one of the blog comment in netlify preview and you can see it in all three url ☺

https://deploy-preview-677--docusaurus-preview.netlify.com/blog/2017/12/14/introducing-docusaurus/index.html

https://deploy-preview-677--docusaurus-preview.netlify.com/blog/2017/12/14/introducing-docusaurus.html

https://deploy-preview-677--docusaurus-preview.netlify.com/blog/2017/12/14/introducing-docusaurus/

Copy link
Contributor

@JoelMarcey JoelMarcey left a comment

Choose a reason for hiding this comment

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

This is awesome! Thanks!

One suggestion. And we can get a second stamp from @yangshun.

@@ -167,6 +167,8 @@ h1 {

`wrapPagesHTML` - Boolean flag to indicate whether `html` files in `/pages` should be wrapped with Docusaurus site styles, header and footer. This feature is experimental and relies on the files being `html` fragments instead of complete pages. It inserts the contents of your `html` file with no extra processing. Defaults to `false`.

`cleanUrl` - If `true`, allow URLs with no `html` extension. Example: request to URL https://docusaurus.io/docs/installation will returns the same result as https://docusaurus.io/docs/installation.html.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this up to be alphabetical with the rest of the optional fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can. But i'm out of reach from my pc until this weekend. I'll do it later if you don't mind. Sorry!

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh - no worries. I can do it for you 👍

@endiliey
Copy link
Contributor Author

endiliey commented May 26, 2018

PR updated. Rebased on top of 1.1.5

Not sure if we want to remove the hardcoded linking values in Docusaurus custom pages if this is merged. Example:
https://github.com/facebook/Docusaurus/blob/b7be85843af2a07ead183dd8a467ebcf83e9a3e3/website/pages/en/index.js#L51

https://github.com/facebook/Docusaurus/blob/b7be85843af2a07ead183dd8a467ebcf83e9a3e3/website/pages/en/index.js#L117

@endiliey endiliey changed the title Enable clean / extension-less url [#413] Enable clean / extension-less url May 28, 2018
@endiliey endiliey changed the title [#413] Enable clean / extension-less url Enable clean / extension-less url Jun 2, 2018
@yangshun
Copy link
Contributor

yangshun commented Jun 2, 2018

@endiliey Could we try to come up with a way to reduce the amount of duplication in the code?

@endiliey
Copy link
Contributor Author

endiliey commented Jun 3, 2018

@yangshun refactor done ! Sorry for the wait. I've tested this on Windows as well.

@JoelMarcey
Copy link
Contributor

Let's do this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants