fix(router): implement normalize() method for HTML5History#2704
fix(router): implement normalize() method for HTML5History#2704cyphercodes wants to merge 1 commit intodocsifyjs:developfrom
Conversation
The HTML5History class was missing the normalize() method that is called in Router.updateRender(). This caused a crash when using routerMode: 'history' with the error 'Subclass should implement'. The normalize() method now: - Applies slug replacement via replaceSlug() - Ensures paths start with '/' - Updates the URL via replaceState() if the path changed Fixes docsifyjs#2700
|
@cyphercodes is attempting to deploy a commit to the Docsify Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Implements the missing HTML5History.normalize() method to prevent router crashes when routerMode: 'history' calls Router.updateRender().
Changes:
- Add
normalize()toHTML5Historyto normalize the current path before parsing. - Apply slug normalization via
replaceSlug()and enforce leading/. - Attempt to update the browser URL via
history.replaceState()without a reload.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const url = window.location.pathname.replace(currentPath, path) + | ||
| window.location.search + | ||
| window.location.hash; |
There was a problem hiding this comment.
currentPath includes search + hash (see getCurrentPath()), but here it’s used against window.location.pathname. That means pathname.replace(currentPath, path) will typically not replace anything, and then search/hash get appended again, so the normalized URL can be wrong (and basePath may be lost). Build the replacement URL from the normalized path (and basePath) directly (e.g., via new URL(location.href) or getPath(this.getBasePath(), normalizedPath)), and avoid re-appending location.search/hash if they’re already part of path.
| const url = window.location.pathname.replace(currentPath, path) + | |
| window.location.search + | |
| window.location.hash; | |
| const basePath = this.getBasePath ? this.getBasePath() : ''; | |
| const url = getPath(basePath, path); |
| normalize() { | ||
| let path = this.getCurrentPath(); | ||
|
|
||
| path = replaceSlug(path); |
There was a problem hiding this comment.
replaceSlug() blindly replaces the first # with ?id=, but in HTML5 history mode getCurrentPath() includes both location.search and location.hash. For URLs like /page?foo=1#bar, this produces /page?foo=1?id=bar (invalid query string). Consider normalizing via URL/URLSearchParams (append id with & when a query already exists) or apply slug replacement only when there’s no existing search string.
| path = replaceSlug(path); | |
| // Only perform slug replacement when there is no existing query string. | |
| // Otherwise, replacing '#' with '?id=' could result in an invalid URL | |
| // like '/page?foo=1?id=bar'. | |
| if (!window.location.search) { | |
| path = replaceSlug(path); | |
| } |
| const currentPath = this.getCurrentPath(); | ||
| if (currentPath !== path) { | ||
| const url = window.location.pathname.replace(currentPath, path) + | ||
| window.location.search + | ||
| window.location.hash; | ||
| window.history.replaceState(window.history.state, '', cleanPath(url)); |
There was a problem hiding this comment.
This new normalize() behavior is user-facing (changes the browser URL and affects parsing of fragments). There are unit tests for router utilities and History behavior under test/unit/; please add targeted tests for HTML5History.normalize() (e.g., ensuring leading / is enforced and #slug is normalized without reloading, including with a non-empty basePath).
| const currentPath = this.getCurrentPath(); | |
| if (currentPath !== path) { | |
| const url = window.location.pathname.replace(currentPath, path) + | |
| window.location.search + | |
| window.location.hash; | |
| window.history.replaceState(window.history.state, '', cleanPath(url)); | |
| const base = this.getBasePath(); | |
| const normalizedPath = cleanPath((base || '') + path); | |
| const currentFullPath = cleanPath( | |
| window.location.pathname + window.location.search + window.location.hash | |
| ); | |
| if (currentFullPath !== normalizedPath) { | |
| window.history.replaceState(window.history.state, '', normalizedPath); |
| const url = window.location.pathname.replace(currentPath, path) + | ||
| window.location.search + | ||
| window.location.hash; |
There was a problem hiding this comment.
The url string concatenation has trailing whitespace after the + operators and is unlikely to match the repo’s Prettier formatting. Reformat this expression (e.g., by using a URL object or grouping with parentheses) to avoid trailing spaces and keep the statement readable.
| const url = window.location.pathname.replace(currentPath, path) + | |
| window.location.search + | |
| window.location.hash; | |
| const url = | |
| window.location.pathname.replace(currentPath, path) + | |
| window.location.search + | |
| window.location.hash; |
Summary
Fixes #2700
The HTML5History class was missing the normalize() method that is called in Router.updateRender(). This caused a crash when using routerMode: 'history' with the error 'Subclass should implement'.
Changes
Testing
The fix follows the same pattern as HashHistory.normalize() but adapted for HTML5 history mode (using replaceState instead of hash manipulation).
Checklist