diff --git a/frontend/composables/useURLParams.ts b/frontend/composables/useURLParams.ts index 631758d..7c247e8 100644 --- a/frontend/composables/useURLParams.ts +++ b/frontend/composables/useURLParams.ts @@ -6,7 +6,49 @@ export const useURLParams = () => { const route = useRoute() const router = useRouter() + // A common bug prior to the pending-values approach was that within a single tick, we'd do multiple updates like + // (1) setVal('foo', 1) + // (2) setVal('bar', 2) + // **router.currentRoute** is a reactive object that **does not get updated between ticks**. + // Thus setVal('foo', 1) would set the value in the URL, but then setVal('bar', 2) would overwrite it. + // Said another way, `setVal('foo', 1)` immediately followed by `getVal('foo'), would return the original value. + // The PendingValues approach solves this by tracking the values that are pending to be set in the URL, + // **and using them as the source of truth when set for `getVal` queries**. + const pendingValues = useState>('useURLParams.pendingValues', () => new Map()) + const hasPendingValues = computed(() => pendingValues.value.size > 0) + + // An interaction pattern we often have is + // (a) change something where the source of truth state is in the URL, then + // (b) reload the core data of the page + // This is a helper method to enable a setter of one or more URL params to wait for them + // to be reflected in the URL Query before initiating + const waitForURLToUpdate = () => { + if (!hasPendingValues.value) { + return Promise.resolve() + } + return new Promise((resolve) => { + // Note, this just lets us know that the value set has been emptied into the URL. + // It doesn't mean that the URL has been updated (i.e. reading currentRoute would yield the incorrect result). + // To sidestep that we watch the currentRoute and wait for it to change AFTER the pending value set is emptied. + const unwatchPV = watch(hasPendingValues, (hpv) => { + if (!hpv) { + unwatchPV() + const unwatchRoute = watch(router.currentRoute, () => { + unwatchRoute() + resolve() + }) + } + }) + }) + } + const getVal = (src: RouteParams | LocationQuery, key: string): string | undefined => { + // NOTE: if the value is in pendingValues, that means it's the source of truth + const pvs = pendingValues.value + if (pvs.has(key)) { + return pvs.get(key) + } + const val = src[key] if (!val) { return undefined @@ -25,18 +67,32 @@ export const useURLParams = () => { return val } - const setVal = (key: string, val: string | undefined) => { + const resolvePendingValues = () => { + const pvs = pendingValues.value const query = new URLSearchParams(stringifyQuery(router.currentRoute.value.query)) - if (val) { - query.set(key, val) - } else { - query.delete(key) + for (const [key, val] of pvs) { + if (val === undefined) { + query.delete(key) + } else { + query.set(key, val) + } } let qs = query.toString() if (qs) { qs = '?' + qs } - void router.replace(qs) + void router.replace(qs).then(() => nextTick(() => { + pendingValues.value = new Map() + })) + } + + const setVal = (key: string, val: string | undefined) => { + pendingValues.value.set(key, val) + // Note: we only try to resolve pending values upon next tick. + // This isn't required, but it prevents us from doing more work on the browser, + // since in multi-update cases, we'll generate multiple replace() calls which will + // become redundant upon the next tick updating the URLs. + void nextTick(resolvePendingValues) } const fromQueryReactive = (key: string): WritableComputedRef => { @@ -69,5 +125,6 @@ export const useURLParams = () => { fromParams: (key: string): string | undefined => { return getVal(route.params, key) }, + waitForURLToUpdate, } }