vuejs / vuepress

📝 Minimalistic Vue-powered static site generator
https://vuepress.vuejs.org
MIT License
22.51k stars 4.76k forks source link

404 on non-English permalinks #2716

Open anisabboud opened 3 years ago

anisabboud commented 3 years ago

Bug report

Steps to reproduce

  1. Create new project: npx create-vuepress-site testvuepresspermalinks cd testvuepresspermalinks/docs

  2. Open VSCode code . and create two files:

src/guide/test1.md:

---
permalink: /english
---
Page 1

src/guide/test2.md:

---
permalink: /עברית
---
Page 2
  1. npm run dev and navigate to http://localhost:8080/english/ http://localhost:8080/עברית/ Both work fine.

  2. Now update VuePress to latest 1.7.1. Stop the server (Ctrl+C), and run npm install -D vuepress@latest (create-vuepress-site used 1.5.3 as we can see from package.json.)

  3. npm run dev again and test again: http://localhost:8080/english/ still works, http://localhost:8080/עברית/ => returns 404.

What is expected?

permalinks should work properly.

What is actually happening?

permalinks with non-English characters show 404 on the latest VuePress. (These are important for SEO...) They used to work fine earlier as we can see from step 3 above...

Further investigation

Upon further investigation, I believe the issue is in the vue-router dependency... Manually installing vue-router 3.4.5 (or any earlier version of it) works fine, but any version 3.4.6+ of vue-router breaks the permalinks.

I.e., if you run npm install -D vue-router@3.4.5 --save-exact && npm run dev the permalinks work fine.

vue-router 3.4.6 was released on 2020-10-07: https://github.com/vuejs/vue-router/blob/dev/CHANGELOG.md#346-2020-10-07

anisabboud commented 3 years ago

The culprit is in https://github.com/vuejs/vue-router/blob/dev/src/history/html5.js#L89 Line 89: let path = window.location.pathname It was: let path = decodeURI(window.location.pathname) but changed two months ago in version 3.4.6 in this commit by @mestevezdeister line changed

Manually editing node_modules/vue-router/dist/vue-router.esm.js (line 2578) and adding decodeURI(...) back fixes the permalinks issue locally (workaround).

Note that I also started getting console warnings introduced in this commit like these: [vue-router] Route with path "/non-english-characters-here" contains unencoded characters, make sure your path is correctly encoded before passing it to the router. Use encodeURI to encode static segments of your path.

Should VuePress handle encoding permalinks and other paths (derived from file/folder names) in response to the recent change by vue-router?

anisabboud commented 3 years ago

Instead of modifying vue-router, this can be fixed on VuePress's side by adding encodeURI in buildPermalink (): https://github.com/vuejs/vuepress/blob/master/packages/%40vuepress/core/lib/node/Page.js#L275 buildPermalink missing encodeURI

That would be consistent with other areas in the same file where encodeURI is already used... encodeURI is already used in the same file

Adding encodeURI locally in my node_modules/@vuepress/core/lib/node/Page.js fixed the 404 issue, and removed one of the warnings by vue-router, but didn't eliminate the second warning... So there might be other location(s) where encodeURI is needed...

One warning solved

King-of-Infinite-Space commented 3 years ago

You may want to check this workaround -- https://github.com/vuepress-reco/vuepress-theme-reco/issues/276#issuecomment-733503061 -- which customizes router in enhanceApp.js

JulioJu commented 3 years ago

Thanks @King-of-Infinite-Space