Skip to content

Commit 5e5631c

Browse files
committed
feat(router): move stackiq off hash routing to clean path URLs
Stackiq is one of seven fleet apps still serving its SPA behind a `#`. This is the pilot for moving all of them: the source change is a single line, and everything else here is the test surface that assumed hashes. Verified BEFORE switching, because history mode fails at the SERVER when the AppHost SPA catch-all is missing (fleet #133 is why apps fell back to hash in the first place): /apps/stackiq/organisaties, /contracten and /organisaties/abc-123 all already returned 200 with the app shell, so the catch-all is present for this app. What moved with it: tests/e2e/spec-coverage/_helpers.ts gotoAppRoute built `${APP_BASE}#${route}` tests/e2e/manifest-pages.spec.ts same URL construction tests/e2e/spec-coverage/catalog-ratings.spec.ts 2 hash deep-links tests/e2e/smoke/app-mounts.spec.ts `/stackiq/#/organisaties` -> a real sub-path The helper docblock explaining vue-router 4's hash-relative `createHref` is rewritten rather than deleted: the id-based nav selector it defends is deliberately KEPT, because identifying the nav by a stable handle instead of an href format the router owns is what makes it survive this change. Verified in the browser against the published @conduction/nextcloud-vue (USE_LOCAL_LIB=false): /apps/stackiq/ -> 200, page id Dashboard, no hash /apps/stackiq/organisaties -> 200, page id Organisaties, no hash /apps/stackiq/contracten -> 200, page id Contracten, no hash RELOAD on /organisaties -> 200, still Organisaties That reload is the point: it is the case hash mode existed to avoid, and it is served by the catch-all rather than 404ing. Zero JS errors. The Playwright smoke project passes on both routes, including the organisations sub-route now that it is a real path. eslint exits 0 and webpack compiles.
1 parent 26d1854 commit 5e5631c

5 files changed

Lines changed: 28 additions & 22 deletions

File tree

src/main.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {
2727
} from '@nextcloud/l10n'
2828
import { generateUrl } from '@nextcloud/router'
2929
import { createApp, h } from 'vue'
30-
import { createRouter, createWebHashHistory } from 'vue-router'
30+
import { createRouter, createWebHistory } from 'vue-router'
3131
import App from './App.vue'
3232
import CatalogPanels from './components/CatalogPanels.vue'
3333
import customComponents from './customComponents.js'
@@ -160,7 +160,14 @@ async function bootstrap() {
160160
)
161161

162162
const router = createRouter({
163-
history: createWebHashHistory(generateUrl('/apps/stackiq')),
163+
// History mode: clean path URLs and working deep-links
164+
// (/apps/stackiq/organisaties/{id}). This relies on the AppHost SPA
165+
// catch-all serving the SPA index on any sub-path — verified before
166+
// the switch: /apps/stackiq/organisaties, /contracten and
167+
// /organisaties/abc-123 all return 200 with the app shell. Without
168+
// that route a deep link 404s at the SERVER on reload, which is the
169+
// reason apps fell back to hash mode (fleet #133).
170+
history: createWebHistory(generateUrl('/apps/stackiq')),
164171
routes: routesFromManifest(resolvedManifest),
165172
})
166173

tests/e2e/manifest-pages.spec.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,9 @@ async function gotoAppRoute(page: Page, route: string): Promise<void> {
6767
// The in-app router runs in hash mode, so deep links are `#<route>`. A bare
6868
// path form boots the SPA but leaves the hash empty, so vue-router falls back
6969
// to the default `/` (Dashboard) and the requested surface never mounts.
70-
// Navigate via the hash; the dashboard is `#/`.
71-
const url = route === '/' ? `${APP_BASE}#/` : `${APP_BASE}#${route}`
70+
// History mode: a deep link is a plain path (the dashboard is `/`).
71+
const base = APP_BASE.endsWith('/') ? APP_BASE.slice(0, -1) : APP_BASE
72+
const url = route === '/' ? `${base}/` : `${base}${route}`
7273
// Use `domcontentloaded`, not `networkidle`: the app fires a periodic
7374
// heartbeat / keep-alive poll, so the network never goes idle and a
7475
// `networkidle` wait times out at 60s. The explicit shell/main waits below

tests/e2e/smoke/app-mounts.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ const ROUTES = [
3737
{ name: 'app root', path: '/index.php/apps/stackiq/' },
3838
{
3939
name: 'organisations sub-route',
40-
path: '/index.php/apps/stackiq/#/organisaties',
40+
path: '/index.php/apps/stackiq/organisaties',
4141
},
4242
]
4343

tests/e2e/spec-coverage/_helpers.ts

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -158,11 +158,13 @@ export async function dismissWalkthrough(page: Page): Promise<void> {
158158

159159
/** Deep-link to a route and wait for the Vue shell + main region to mount. */
160160
export async function gotoAppRoute(page: Page, route: string): Promise<void> {
161-
// The in-app router runs in hash mode, so deep links are `#<route>`. A bare
162-
// path form (e.g. `/apps/stackiq/settings`) boots the SPA but leaves
163-
// the hash empty, so vue-router falls back to the default `/` (Dashboard)
164-
// and the requested surface never mounts. Always navigate via the hash.
165-
const url = route === '/' ? `${APP_BASE}#/` : `${APP_BASE}#${route}`
161+
// The in-app router runs in HISTORY mode, so a deep link is a plain path.
162+
// This works only because the AppHost SPA catch-all serves the app shell on
163+
// any sub-path; if that route ever goes missing these navigations 404 at the
164+
// server rather than falling back to the dashboard, which is the loud
165+
// failure we want.
166+
const base = APP_BASE.endsWith('/') ? APP_BASE.slice(0, -1) : APP_BASE
167+
const url = route === '/' ? `${base}/` : `${base}${route}`
166168
await page.goto(url, { waitUntil: 'domcontentloaded' })
167169
await page
168170
.locator(APP_SHELL)
@@ -183,16 +185,12 @@ export async function gotoAppRoute(page: Page, route: string): Promise<void> {
183185
* check below is unchanged in strength.
184186
*
185187
* ⚠️ This used to be `nav:has(a[href*="/apps/stackiq/"])`, which stopped
186-
* matching ANYTHING under vue-router 4. In hash mode v4 emits HASH-RELATIVE
187-
* hrefs (`#/organisaties`); vue-router 3 emitted the base too
188-
* (`/apps/stackiq/#/organisaties`). v4's `createHref` explicitly strips
189-
* everything before the `#`, so no configuration of `createWebHashHistory`
190-
* restores the old shape — the change is by design, not a misconfiguration.
191-
*
192-
* Navigation itself is unaffected: `#/organisaties` resolves against the current
193-
* document, the click navigates, and the target page renders. Verified in a
194-
* browser before this selector was touched, precisely so that a stale selector
195-
* could not be "fixed" into hiding a real routing regression.
188+
* matching ANYTHING under vue-router 4 in HASH mode, because v4's `createHref`
189+
* strips everything before the `#` and emits hash-relative hrefs
190+
* (`#/organisaties`) where v3 emitted the base too. The app has since moved to
191+
* history mode, so full-path hrefs are back — but the id-based selector below
192+
* is kept deliberately: it identifies the element by a stable handle rather
193+
* than by an href format the router owns, and so survives the next such change.
196194
*
197195
* `nav#app-navigation-vue` is @nextcloud/vue's own NcAppNavigation host and is
198196
* unique on the page (the other two navs are core's app-menu and user-menu), so

tests/e2e/spec-coverage/catalog-ratings.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ async function newAnonymousContext(): Promise<APIRequestContext> {
136136

137137
/** Open the seeded module's detail page and wait for the reviews panel. */
138138
async function openModuleReviews(page: Page): Promise<void> {
139-
await page.goto(`${APP_BASE}#/modules/${moduleUuid}`, {
139+
await page.goto(`${APP_BASE.replace(/\/$/, "")}/modules/${moduleUuid}`, {
140140
waitUntil: 'domcontentloaded',
141141
})
142142
await page
@@ -379,7 +379,7 @@ test('reviews: a module with no approved reviews shows the empty aggregate, not
379379
})
380380
expect(uuid, 'isolated module fixture has no uuid').not.toBe('')
381381

382-
await page.goto(`${APP_BASE}#/modules/${uuid}`, {
382+
await page.goto(`${APP_BASE.replace(/\/$/, "")}/modules/${uuid}`, {
383383
waitUntil: 'domcontentloaded',
384384
})
385385
await page

0 commit comments

Comments
 (0)