Merge pull request #911 from stevapple/browser-router

feat(web): adopt browser routing for cleaner URIs

Dockerfile

@@ -1,5 +1,5 @@
 # building frontend
-FROM node:22-slim as frontend
+FROM node:22-slim AS frontend
 WORKDIR /app/frontend
 
 COPY web/package.json web/yarn.lock ./
@@ -40,8 +40,8 @@ FROM python:3.12-slim
 ENV MAX_UPLOAD_SIZE=100M
 
 # set up the system
-RUN apt update && \
-    apt install --yes nginx dumb-init libmagic1 gettext && \
+RUN apt-get update && \
+    apt-get install --yes nginx dumb-init libmagic1 gettext && \
     rm -rf /var/lib/apt/lists/*
 
 RUN mkdir -p /var/docat/doc

docat/docat/nginx/default

@@ -15,6 +15,7 @@ server {
 
     location /doc {
         root /var/docat;
+        absolute_redirect off;
     }
 
     location /api {
@@ -23,6 +24,6 @@ server {
     }
 
     location / {
-        try_files $uri $uri/ =404;
+        try_files $uri $uri/ /index.html =404;
     }
 }

docat/docat/utils.py

@@ -126,7 +126,7 @@ def is_forbidden_project_name(name: str) -> bool:
     a page on the docat website.
     """
     name = name.lower().strip()
-    return name in ["upload", "claim", "delete", "help"]
+    return name in ["upload", "claim", "delete", "help", "doc", "api"]
 
 
 UNITS_MAPPING = [

docat/tests/test_rename.py

@@ -86,7 +86,7 @@ def test_rename_rejects_forbidden_project_name(client_with_claimed_project):
     assert create_response.status_code == 201
 
     with patch("os.rename") as rename_mock:
-        for project_name in ["upload", "claim", "Delete ", "help"]:
+        for project_name in ["upload", "claim", "Delete ", "help", "Doc", "API"]:
             rename_response = client_with_claimed_project.put(f"/api/some-project/rename/{project_name}", headers={"Docat-Api-Key": "1234"})
             assert rename_response.status_code == 400
             assert rename_response.json() == {

docat/tests/test_upload.py

@@ -160,7 +160,7 @@ def test_upload_rejects_forbidden_project_name(client_with_claimed_project):
     """
 
     with patch("docat.app.remove_docs") as remove_mock:
-        for project_name in ["upload", "claim", " Delete ", "help"]:
+        for project_name in ["upload", "claim", " Delete ", "help", "DOC", "api"]:
             response = client_with_claimed_project.post(
                 f"/api/{project_name}/1.0.0", files={"file": ("index.html", io.BytesIO(b"<h1>Hello World</h1>"), "plain/text")}
             )

web/src/App.tsx

@@ -1,4 +1,4 @@
-import { createHashRouter, RouterProvider } from 'react-router-dom'
+import { createBrowserRouter, RouterProvider } from 'react-router-dom'
 import { ConfigDataProvider } from './data-providers/ConfigDataProvider'
 import { MessageBannerProvider } from './data-providers/MessageBannerProvider'
 import { ProjectDataProvider } from './data-providers/ProjectDataProvider'
@@ -7,14 +7,13 @@ import { StatsDataProvider } from './data-providers/StatsDataProvider'
 import Claim from './pages/Claim'
 import Delete from './pages/Delete'
 import Docs from './pages/Docs'
-import EscapeSlashForDocsPath from './pages/EscapeSlashForDocsPath'
 import Help from './pages/Help'
 import Home from './pages/Home'
 import NotFound from './pages/NotFound'
 import Upload from './pages/Upload'
 
 function App(): JSX.Element {
-  const router = createHashRouter([
+  const router = createBrowserRouter([
     {
       path: '/',
       errorElement: <NotFound />,
@@ -54,17 +53,8 @@ function App(): JSX.Element {
                   element: <Docs />
                 },
                 {
-                  path: ':page',
-                  children: [
-                    {
-                      path: '',
-                      element: <Docs />
-                    },
-                    {
-                      path: '*',
-                      element: <EscapeSlashForDocsPath />
-                    }
-                  ]
+                  path: '*',
+                  element: <Docs />
                 }
               ]
             }

web/src/components/DocumentControlButtons.tsx

@@ -41,8 +41,10 @@ export default function DocumentControlButtons(props: Props): JSX.Element {
       url = url.replace(props.version, 'latest')
     }
 
-    if (shareModalHideUi && !url.includes('?hide-ui=true')) {
-      url = `${url}?hide-ui=true`
+    if (shareModalHideUi) {
+      const urlObject = new URL(url)
+      urlObject.search = 'hide-ui'
+      url = urlObject.toString()
     }
 
     return url

web/src/pages/Docs.tsx

@@ -1,4 +1,4 @@
-import React, { useEffect, useMemo, useState, useRef } from 'react'
+import { useEffect, useMemo, useState, useRef } from 'react'
 import ProjectRepository from '../repositories/ProjectRepository'
 import type ProjectDetails from '../models/ProjectDetails'
 import LoadingPage from './LoadingPage'
@@ -11,7 +11,7 @@ import IframeNotFound from './IframePageNotFound'
 
 export default function Docs(): JSX.Element {
   const params = useParams()
-  const searchParams = useSearchParams()[0]
+  const [searchParams] = useSearchParams()
   const location = useLocation()
   const { showMessage, clearMessages } = useMessageBanner()
 
@@ -20,14 +20,11 @@ export default function Docs(): JSX.Element {
   const [loadingFailed, setLoadingFailed] = useState<boolean>(false)
 
   const project = useRef(params.project ?? '')
-  const page = useRef(params.page ?? 'index.html')
-  const hash = useRef(location.hash.split('?')[0] ?? '')
+  const page = useRef(params['*'] ?? '')
+  const hash = useRef(location.hash)
 
   const [version, setVersion] = useState<string>(params.version ?? 'latest')
-  const [hideUi, setHideUi] = useState<boolean>(
-    searchParams.get('hide-ui') === 'true' ||
-      location.hash.split('?')[1] === 'hide-ui=true'
-  )
+  const [hideUi, setHideUi] = useState<boolean>(searchParams.get('hide-ui') === '' || searchParams.get('hide-ui') === 'true')
   const [iframeUpdateTrigger, setIframeUpdateTrigger] = useState<number>(0)
 
   // This provides the url for the iframe.
@@ -91,10 +88,9 @@ export default function Docs(): JSX.Element {
     })()
   }, [project])
 
-  /** Encodes the url for the current page, and escapes the path part to avoid
-   * redirecting to escapeSlashForDocsPath.
+  /** Encodes the url for the current page.
    * @example
-   * getUrl('project', 'version', 'path/to/page.html', '#hash', false) -> '#/project/version/path%2Fto%2Fpage.html#hash'
+   * getUrl('project', 'version', 'path/to/page.html', '#hash', false) -> '/project/version/path/to/page.html#hash'
    */
   const getUrl = (
     project: string,
@@ -103,7 +99,7 @@ export default function Docs(): JSX.Element {
     hash: string,
     hideUi: boolean
   ): string => {
-    return `#/${project}/${version}/${encodeURIComponent(page)}${hash}${hideUi ? '?hide-ui=true' : ''}`
+    return `/${project}/${version}/${encodeURI(page)}${hash}${hideUi ? '?hide-ui' : ''}`
   }
 
   const updateUrl = (newVersion: string, hideUi: boolean): void => {
@@ -121,6 +117,11 @@ export default function Docs(): JSX.Element {
     document.title = newTitle
   }
 
+  // Keep compatibility with encoded page path
+  useEffect(() => {
+    updateUrl(version, hideUi)
+  }, [])
+
   const iFramePageChanged = (urlPage: string, urlHash: string, title?: string): void => {
     if (title != null && title !== document.title) {
       updateTitle(title)
@@ -156,11 +157,9 @@ export default function Docs(): JSX.Element {
   useEffect(() => {
     const urlProject = params.project ?? ''
     const urlVersion = params.version ?? 'latest'
-    const urlPage = params.page ?? 'index.html'
-    const urlHash = location.hash.split('?')[0] ?? ''
-    const urlHideUi =
-      searchParams.get('hide-ui') === 'true' ||
-      location.hash.split('?')[1] === 'hide-ui=true'
+    const urlPage = params['*'] ?? ''
+    const urlHash = location.hash
+    const urlHideUi = searchParams.get('hide-ui') === '' || searchParams.get('hide-ui') === 'true'
 
     // update the state to the url params on first loadon
     if (urlProject !== project.current) {

web/src/pages/Home.tsx

@@ -1,7 +1,7 @@
 import { useEffect, useState } from 'react';
+import { useNavigate } from 'react-router';
 
 import { Delete, ErrorOutline, FileUpload, KeyboardArrowDown, Lock } from '@mui/icons-material';
-import { useLocation } from 'react-router';
 import { useProjects } from '../data-providers/ProjectDataProvider';
 import { useSearch } from '../data-providers/SearchProvider';
 import { type Project } from '../models/ProjectsResponse';
@@ -20,26 +20,19 @@ import styles from './../style/pages/Home.module.css';
 
 
 export default function Home(): JSX.Element {
+  const navigate = useNavigate()
   const { loadingFailed } = useProjects()
   const { stats, loadingFailed: statsLoadingFailed } = useStats()
-  const { filteredProjects: projects, query, setQuery } = useSearch()
+  const { filteredProjects: projects, query } = useSearch()
   const [showAll, setShowAll] = useState(false);
   const [favoriteProjects, setFavoriteProjects] = useState<Project[]>([])
 
-  const location = useLocation()
-
   document.title = 'Home | docat'
 
-  // insert # into the url if it's missing
-  useEffect(() => {
-    const nonHostPart = window.location.href.replace(window.location.origin, '')
-
-    if (nonHostPart.startsWith('#') || nonHostPart.startsWith('/#')) {
-      return
-    }
-
-    window.location.replace(`/#${nonHostPart}`)
-  }, [location, setQuery, projects])
+  // Keep compatibility with hash-based URI
+  if (location.hash.startsWith('#/')) {
+    navigate(location.hash.replace('#', ''), { replace: true })
+  }
 
   const updateFavorites = (): void => {
     if (projects == null) return

web/src/pages/IframePageNotFound.tsx

@@ -11,7 +11,7 @@ interface Props {
 }
 
 export default function IframeNotFound(props: Props): JSX.Element {
-  const link = `/${props.project}/${props.version}${props.hideUi ? '?hide-ui=true' : ''}`
+  const link = `/${props.project}/${props.version}${props.hideUi ? '?hide-ui' : ''}`
 
   return (
     <div className={styles['iframe-page-not-found']}>

web/src/repositories/ProjectRepository.ts

@@ -4,34 +4,6 @@ import { type Project } from '../models/ProjectsResponse'
 
 const RESOURCE = 'doc'
 
-/**
- * Escapes all slashes in a url to the docs page from the point between the version and the path.
- * This is necessary because react-router thinks that the slashes are path separators.
- * The slashes are escaped to %2F and reverted back to slashes by react-router.
- * Example:
- *  /doc/project/1.0.0/path/to/page -> /doc/project/1.0.0/path%2Fto%2Fpage
- * @param pathname useLocation().pathname
- * @param search useLocation().search
- * @param hash useLocation().hash
- * @returns a url with escaped slashes
- */
-function escapeSlashesInUrl(
-  pathname: string,
-  search: string,
-  hash: string
-): string {
-  const url = pathname + hash + search
-  const projectAndVersion = url.split('/', 3).join('/')
-  let path = url.substring(projectAndVersion.length + 1)
-  path = path.replaceAll('/', '%2F')
-
-  if (path.length === 0) {
-    return projectAndVersion
-  }
-
-  return projectAndVersion + '/' + path
-}
-
 function dateTimeReviver(key: string, value: any) {
   if (key === 'timestamp') {
     return new Date(value)
@@ -273,7 +245,6 @@ function setFavorite(projectName: string, shouldBeFavorite: boolean): void {
 }
 
 const exp = {
-  escapeSlashesInUrl,
   getVersions,
   getLatestVersion,
   filterHiddenVersions,

web/src/tests/repositories/ProjectRepository.test.ts

@@ -473,54 +473,3 @@ describe('getLatestVersion', () => {
     expect(latestVersion).toStrictEqual(versions[0])
   })
 })
-
-describe('escapeSlashesInUrl', () => {
-  test('should ignore version and project name', () => {
-    const url = '/project/1.0.0'
-
-    expect(ProjectRepository.escapeSlashesInUrl(url, '', '')).toBe(url)
-  })
-
-  test('should ignore trailing slash', () => {
-    const given = '/project/1.0.0/'
-    const expected = '/project/1.0.0'
-
-    expect(ProjectRepository.escapeSlashesInUrl(given, '', '')).toBe(expected)
-  })
-
-  test('should escape slashes in path', () => {
-    const given = '/project/1.0.0/path/with/slashes'
-    const expected = '/project/1.0.0/path%2Fwith%2Fslashes'
-
-    expect(ProjectRepository.escapeSlashesInUrl(given, '', '')).toBe(expected)
-  })
-
-  test('should work with query parameters', () => {
-    const given = '/project/1.0.0/path/with/slashes'
-    const query = '?param=value'
-    const expected = '/project/1.0.0/path%2Fwith%2Fslashes?param=value'
-
-    expect(ProjectRepository.escapeSlashesInUrl(given, query, '')).toBe(
-      expected
-    )
-  })
-
-  test('should work with hash', () => {
-    const given = '/project/1.0.0/path/with/slashes'
-    const hash = '#hash'
-    const expected = '/project/1.0.0/path%2Fwith%2Fslashes#hash'
-
-    expect(ProjectRepository.escapeSlashesInUrl(given, '', hash)).toBe(expected)
-  })
-
-  test('should work with query parameters and hash', () => {
-    const given = '/project/1.0.0/path/with/slashes'
-    const query = '?param=value'
-    const hash = '#hash'
-    const expected = '/project/1.0.0/path%2Fwith%2Fslashes#hash?param=value'
-
-    expect(ProjectRepository.escapeSlashesInUrl(given, query, hash)).toBe(
-      expected
-    )
-  })
-})