vuejs / vitepress

Vite & Vue powered static site generator.
https://vitepress.dev
MIT License
11.48k stars 1.86k forks source link

feat: add `cleanUrls` option (#219) #869

Closed georges-gomes closed 1 year ago

georges-gomes commented 1 year ago

( Replaces #488 for alpha)

This feature is meant to be working with SPA and MPA mode so it's a little bit bigger than just rename the browser location.


fixes #219, fixes #444

brc-dd commented 1 year ago

@kiaking I was thinking if instead of providing an option for cleanUrls, we can make it default? It'll simplify the code much more (like there will not be any need to pass it through different functions to normalizeHref). Also, this should fix our Algolia issue 😅.

@georges-gomes Is there any compelling reason that people would want to set cleanUrls to false?

georges-gomes commented 1 year ago

@brc-dd for people already using VitePress, this would change their URLs... It has a number of impacts for them. We can have cleanUrls default to true for v1.0.0 (not my call) but I think we need this option for migration.

georges-gomes commented 1 year ago

@kiaking Does it looks good to you? Let me know if I should adjust something.

azat-io commented 1 year ago

Awesome 😍

brc-dd commented 1 year ago

There is .html inside certain docs too. Since we're not automatically removing html from pages, it will be better to have consistent URLs in the docs. Also, it appears you've not allowed edits by maintainers in the PR.

These docs had this https://github.com/vuejs/vitepress/pull/724/files#diff-d019d98f93ab320a29453160fbb723955227add7e423571649818128db30c595, https://github.com/vuejs/vitepress/pull/724/files#diff-3193bd261794eacfc0903b854591814239fe7c7737175464622570a374b517db. There might be a few more now.

brc-dd commented 1 year ago

Not sure, but we might need to change this too:

https://github.com/vuejs/vitepress/blob/753f35bf7c2a24bd37f6f05a5e13804b01dc512b/src/client/app/router.ts#L251

brc-dd commented 1 year ago

This issue is also there: #412 if you add trailing slash to your URL (shows title 404, etc.). I had fixed this in vite-3 branch itself.

brc-dd commented 1 year ago

Also, there is some issue with sidebar. Open this: https://deploy-preview-869--vitepress-docs.netlify.app/guide/getting-started/, now click on configuration in sidebar. URL becomes this: https://deploy-preview-869--vitepress-docs.netlify.app/guide/configuration.html. Reload page it becomes: https://deploy-preview-869--vitepress-docs.netlify.app/guide/configuration/.

Sorry, the content is not changing. Just the URL is. (and the title)

georges-gomes commented 1 year ago

Also, there is some issue with sidebar. Open this: https://deploy-preview-869--vitepress-docs.netlify.app/guide/getting-started/, now click on configuration in sidebar. URL becomes this: https://deploy-preview-869--vitepress-docs.netlify.app/guide/configuration.html. Reload page it becomes: https://deploy-preview-869--vitepress-docs.netlify.app/guide/configuration/.

This is new, I have to check why. Thanks

brc-dd commented 1 year ago

You need to change this normalizedPath thing for sidebar issues IG:

https://github.com/vuejs/vitepress/pull/724/files#diff-4eb03faf823699119c94556187e2edb5771912371c55f9aef88804ec6c996564

georges-gomes commented 1 year ago

You need to change this normalizedPath thing for sidebar issues IG:

https://github.com/vuejs/vitepress/pull/724/files#diff-4eb03faf823699119c94556187e2edb5771912371c55f9aef88804ec6c996564

Should be much better now.

georges-gomes commented 1 year ago

@kiaking @brc-dd when you are happy with this change we can switch off the cleanUrls option before merging if you don't want it for the production site. But since we are bringing a brand new documentation, it would be the moment to bring this on... Your call.

brc-dd commented 1 year ago

when you are happy with this change we can switch off the cleanUrls option before merging if you don't want it for the production site. But since we are bringing a brand new documentation, it would be the moment to bring this on... Your call.

Yeah, there is no need to disable it. 👍

brc-dd commented 1 year ago

Can you target this to chore/vite-3 branch for now? Some fixes are not on main currently. That's why reloading pages is giving 404 with hydration mismatch.

brc-dd commented 1 year ago

@georges-gomes Without trailing slash Netlify is having issue. Open this site for example: https://62c2e731bab5b3761e2ce010--fastidious-stroopwafel-625e87.netlify.app/guide/getting-started

georges-gomes commented 1 year ago

@georges-gomes Without trailing slash Netlify is having issue. Open this site for example: https://62c2e731bab5b3761e2ce010--fastidious-stroopwafel-625e87.netlify.app/guide/getting-started

Is Netlify setup with "Pretty URLs" ?

brc-dd commented 1 year ago

Is Netlify setup with "Pretty URLs" ?

No, asset optimization is disabled.

brc-dd commented 1 year ago

Having trailing slash will cause other issues like a URL or asset like this ./theme-introduction will resolve incorrectly. And Netlify apparently adds that:

With a file at /blog.html

  • Navigating to /blog.html will serve /blog.html
  • Navigating to /blog will serve /blog
  • Navigating to /blog/ will 301 redirect and serve /blog

With a file at /blog/index.html (and no file at /blog.html!)

  • Navigating to /blog.html will 301 redirect and serve /blog/
  • Navigating to /blog will 301 redirect and serve /blog/
  • Navigating to /blog/ will serve /blog/

What's the solution here? :/

georges-gomes commented 1 year ago

When I use npx serve or Firebase hosting, I don't have issues. But we need to find the proper setup for Netlify I agree

brc-dd commented 1 year ago

The issue is on Vercel too, but on Vercel we can set trailingSlash redirects, so that fixes the stuff there.

georges-gomes commented 1 year ago

I'm reading that Netlify is biais toward trailing slash in our configuration. I think I'm going to option trailing and non-trailing slash option depending on the hosting solution. Nothing is easy.

georges-gomes commented 1 year ago

I created the option

cleanUrls

When set to "off", page foo/bar.md is generated into foo/bar.html.

When set to "with-trailing-slash" or "without-trailing-slash", page foo/bar.md is generated into foo/bar/index.html.

When set to "with-trailing-slash", URLs will be foo/bar/. When set to "without-trailing-slash", URLs will be foo/bar.

Notes:

georges-gomes commented 1 year ago

But there is still an issue with the router. It doesn't like trailing slashes. I couldn't find the solution quickly.

georges-gomes commented 1 year ago

OK trailing slash is fixed.

But there is a little challenge:

When with-trailing-slash is on, URL foo/bar/ could be resolved to file foo/bar.md or foo/bar/index.md. For the moment I hardcoded the resolve to foo/bar.md...

The without-trailing-slash option doesn't have this problem.

brc-dd commented 1 year ago

There are some more issues with clean urls. Like we have relative links in markdown. Click links in the "what's next" section on this page: http://localhost:4173/guide/getting-started/. They aren't being properly resolved. Also prefetching links is not working (see console for errors as you scroll to the bottom). I suppose the links can be handled in the markdown plugin. But there would be a challenge in determining if the page is index (no need to modify urls in this case) or if its just foo which we have made foo/index (will need to increase one level). Also this won't handle links inside Vue components. Also vitepress dev is not working. This is getting a lot more complex than I expected. 🥲

georges-gomes commented 1 year ago

It's starting to require a lot of code to deal with trailing slash... We better just not support it... and keep the option off while VitePress doc is hosted on Netlify...

brc-dd commented 1 year ago

I am thinking lets have three options:

  1. Don't nest directories (no trailing slash) (just the router stuff, will work on almost all of the popular hosting services including Netlify, Vercel, GitHub Pages, Cloudflare Pages, cPanel/Apache/Nginx based hosting providers with 1-2 line config).

  2. Nest directories (no trailing slash) will work with many hosting providers, especially the CDN type / static ones.

  3. Off

Regarding the MPA mode, not nesting will work if the server is configured to rewrite the URLs, otherwise it won't. It will work in MPA if nested.

georges-gomes commented 1 year ago

I don't understand what you call "nest directories" and "don't nest directory"

brc-dd commented 1 year ago

I meant this:

/foo.md -> /foo/index.html (create clean directory structure)) /foo.md -> /foo.html (don't create clean directory structure)

georges-gomes commented 1 year ago

I don't know if we can do (1) - it would rely of the hosting service to sert /foo.html when requested /foo without redirect... I can leave the door open for (1) but just implement (2) and (3) for now.

brc-dd commented 1 year ago

Fine! 👍 BTW, VuePress also has this warning in place:

This plugin will always work on your dev server, but VuePress does not have the right to modify server identification. If you want your URLs to follow a certain pattern (e.g. /routing instead of /routing.html or routing/), you should make sure that your server would treat it as an HTML. This means that you may need to configure your server specifically.

georges-gomes commented 1 year ago

Done, with nicer documentation. Switched off in the PR so the VitePress documentation works great on Netlify.

brc-dd commented 1 year ago

Okay so I've merged these changes to the Vite3 branch with certain changes. Thanks for all the work!! 💚

brc-dd commented 1 year ago

Can you review #856 and verify if the clean urls are working cool with with-subfolders option? I had tested it on my setup. It worked fine. Just making sure twice.

georges-gomes commented 1 year ago

Can you review #856 and verify if the clean urls are working cool with with-subfolders option? I had tested it on my setup. It worked fine. Just making sure twice.

Testing soon!