Skip to content

Commit 328db56

Browse files
authored
feat(router): move stackiq off hash routing to clean path URLs (#899)
* 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. * fix(router): derive the router base from the served URL History mode broke deep links on the /index.php/... URL form. Nextcloud serves the same app under BOTH /apps/stackiq/... and /index.php/apps/stackiq/..., but generateUrl() returns only the form the instance is configured for. Arriving on the other form left the path outside the router base, vue-router could not resolve it, and the catch-all redirected to '/' -- the visitor landed on the Dashboard with no error and the deep link was silently swallowed. Measured before the fix: /apps/stackiq/komplianties -> Compliance /index.php/apps/stackiq/komplianties -> Dashboard <- silently wrong Hash routing never had this: the route travelled in the fragment, so the path prefix was irrelevant. This is the one real regression the switch introduced, and it would have applied to every app in the rollout. The e2e suite navigates via /index.php/..., which is exactly how it was caught -- two index specs failed while manual browsing on the pretty URL looked fine. Now both forms resolve, with the path preserved.
1 parent 832633f commit 328db56

5 files changed

Lines changed: 56 additions & 22 deletions

File tree

‎src/main.js‎

Lines changed: 37 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'
@@ -130,6 +130,34 @@ const pageTypesProp = { ...defaultPageTypes }
130130
const customComponentsProp = { ...customComponents }
131131
const registryProp = { ...registry }
132132

133+
/**
134+
* The router base for THIS page load.
135+
*
136+
* ⚠️ `generateUrl('/apps/stackiq')` alone is not enough. Nextcloud serves the
137+
* same app under BOTH `/apps/stackiq/...` and `/index.php/apps/stackiq/...`,
138+
* but `generateUrl()` returns only the form the instance is configured for. If
139+
* a visitor arrives on the other form — a bookmark, an emailed deep link, an
140+
* integration that hardcodes `/index.php` — the path no longer starts with the
141+
* router base, vue-router cannot resolve it, and the catch-all redirects to
142+
* `/`. The user lands on the Dashboard with no error: the deep link is
143+
* silently swallowed.
144+
*
145+
* Hash routing never had this, because the route travelled in the fragment and
146+
* the path prefix was irrelevant. Measured on this app before the fix:
147+
* `/apps/stackiq/komplianties` rendered Compliance, while
148+
* `/index.php/apps/stackiq/komplianties` rendered the Dashboard — and that is
149+
* the form the e2e suite uses, which is how it was caught.
150+
*
151+
* So derive the base from the URL actually being served, falling back to
152+
* `generateUrl()` when the app segment is absent.
153+
*
154+
* @return {string} The base path vue-router should strip from the URL.
155+
*/
156+
function routerBase() {
157+
const match = window.location.pathname.match(/^(.*\/apps\/stackiq)(?:\/|$)/)
158+
return match ? match[1] : generateUrl('/apps/stackiq')
159+
}
160+
133161
/**
134162
* Resolve `@resolve:<key>` IAppConfig sentinels in `manifest.pages[].config`
135163
* (e.g. `@resolve:voorzieningen_register`) APP-SIDE, before the router and
@@ -160,7 +188,14 @@ async function bootstrap() {
160188
)
161189

162190
const router = createRouter({
163-
history: createWebHashHistory(generateUrl('/apps/stackiq')),
191+
// History mode: clean path URLs and working deep-links
192+
// (/apps/stackiq/organisaties/{id}). This relies on the AppHost SPA
193+
// catch-all serving the SPA index on any sub-path — verified before
194+
// the switch: /apps/stackiq/organisaties, /contracten and
195+
// /organisaties/abc-123 all return 200 with the app shell. Without
196+
// that route a deep link 404s at the SERVER on reload, which is the
197+
// reason apps fell back to hash mode (fleet #133).
198+
history: createWebHistory(routerBase()),
164199
routes: routesFromManifest(resolvedManifest),
165200
})
166201

‎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)