Skip to content

Commit 6d53b65

Browse files
committed
fix(frontend): address PR review findings (round 3)
- useColorScheme: install the data-scheme watcher in a detached effectScope so it lives for the app lifetime, decoupled from whichever component first calls useColorScheme (previously it was implicitly tied to that caller's component scope and would stop if that component unmounted) - useAuth: document that login(returnUrl) takes a relative/same-origin path only, to avoid widening the open-redirect surface - ci.yml: build the admin frontend via the directory form (-w packages/admin/ frontend-src), consistent with the frontend test step
1 parent f0e6bd5 commit 6d53b65

3 files changed

Lines changed: 17 additions & 4 deletions

File tree

.github/workflows/ci.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,8 @@ jobs:
571571

572572
- name: Build admin frontend
573573
if: matrix.package == 'admin' && needs.changes.outputs.admin == 'true'
574-
run: npm run build -w regelrecht-admin-ui
574+
# Directory form, consistent with the frontend test step above.
575+
run: npm run build -w packages/admin/frontend-src
575576

576577
- name: Skip (no relevant changes)
577578
if: needs.changes.outputs[matrix.package] != 'true'

packages/frontend-shared/src/useAuth.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ export function useAuth() {
3737
// Accepts an explicit return URL so callers that know the user's intended
3838
// destination (e.g. a router guard firing before navigation commits, where
3939
// `window.location` still points at the source route) can forward it to
40-
// SSO. Falls back to the current location for the common case.
40+
// SSO. Falls back to the current location for the common case. Pass a
41+
// relative/same-origin path only: `returnUrl` is forwarded verbatim as the
42+
// SSO `return_url` (the backend validates it, but don't widen the surface by
43+
// handing it an absolute external URL).
4144
function login(returnUrl) {
4245
// Only accept an explicit string: a template that passes `login` as a
4346
// bare event handler (`@click="login"`) hands us a PointerEvent, which

packages/frontend-shared/src/useColorScheme.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
*
1717
* A persistence adapter is `{ theme: Ref<string>, setTheme(value): void }`.
1818
*/
19-
import { ref, readonly, watch } from 'vue';
19+
import { ref, readonly, watch, effectScope } from 'vue';
2020

2121
export const VALID_THEMES = ['auto', 'light', 'dark'];
2222

@@ -75,6 +75,13 @@ let defaultPersistence = null;
7575
// so repeated `useColorScheme` calls don't stack duplicate watchers.
7676
const appliedBackends = new WeakSet();
7777

78+
// Detached scope so the data-scheme watcher lives for the app's lifetime,
79+
// independent of which component's setup() first calls `useColorScheme`. Without
80+
// this the watcher would be owned by that first caller's component scope and
81+
// would be auto-stopped if that component ever unmounts, silently killing the
82+
// applier.
83+
const applierScope = effectScope(true);
84+
7885
/**
7986
* @param {{ theme: import('vue').Ref<string>, setTheme: (v: string) => void }} [persistence]
8087
* Optional persistence adapter. Defaults to a shared localStorage adapter.
@@ -88,7 +95,9 @@ export function useColorScheme(persistence) {
8895
if (!appliedBackends.has(backend)) {
8996
appliedBackends.add(backend);
9097
// `immediate` applies the current (cached/default) theme now and on change.
91-
watch(backend.theme, applyColorScheme, { immediate: true });
98+
// Run inside the detached scope so the watcher isn't tied to the caller's
99+
// component lifetime.
100+
applierScope.run(() => watch(backend.theme, applyColorScheme, { immediate: true }));
92101
}
93102

94103
return {

0 commit comments

Comments
 (0)