From dbc778ac65b92e9bb6fe4459e223e8d1c7d9dc52 Mon Sep 17 00:00:00 2001 From: Taylor Downs Date: Fri, 29 May 2026 12:52:49 +0200 Subject: [PATCH 1/4] don't write no-ops as new URL history items --- assets/js/react/lib/use-url-state.ts | 4 +++ assets/test/react/lib/use-url-state.test.ts | 37 ++++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/assets/js/react/lib/use-url-state.ts b/assets/js/react/lib/use-url-state.ts index a7a915e266b..348ced28414 100644 --- a/assets/js/react/lib/use-url-state.ts +++ b/assets/js/react/lib/use-url-state.ts @@ -102,6 +102,10 @@ class URLStore { const newURL = new URL(window.location.pathname, window.location.origin); newURL.search = newParams.toString(); newURL.hash = window.location.hash; + // Skip no-op writes so mount-time normalization doesn't stack + // duplicate browser history entries (a no-op pushState never notifies + // subscribers anyway, due to the guard in updateParams). + if (newURL.href === window.location.href) return; history.pushState({}, '', newURL); }; diff --git a/assets/test/react/lib/use-url-state.test.ts b/assets/test/react/lib/use-url-state.test.ts index 2464c4d7ec4..4dc5c68ae06 100644 --- a/assets/test/react/lib/use-url-state.test.ts +++ b/assets/test/react/lib/use-url-state.test.ts @@ -11,7 +11,7 @@ */ import { renderHook, act } from '@testing-library/react'; -import { describe, test, expect, beforeEach } from 'vitest'; +import { describe, test, expect, beforeEach, afterEach, vi } from 'vitest'; import { useURLState } from '../../../js/react/lib/use-url-state'; @@ -201,6 +201,41 @@ describe('useURLState', () => { }); }); + describe('updateSearchParams - no-op writes', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + test('does not push a history entry when params are unchanged', () => { + history.replaceState({}, '', '/workflow?panel=run&job=abc'); + + const { result } = renderHook(() => useURLState()); + const pushSpy = vi.spyOn(history, 'pushState'); + + act(() => { + // Same values that are already in the URL -> a no-op + result.current.updateSearchParams({ panel: 'run', job: 'abc' }); + }); + + expect(pushSpy).not.toHaveBeenCalled(); + expect(window.location.search).toBe('?panel=run&job=abc'); + }); + + test('still pushes a history entry when a param actually changes', () => { + history.replaceState({}, '', '/workflow?panel=run'); + + const { result } = renderHook(() => useURLState()); + const pushSpy = vi.spyOn(history, 'pushState'); + + act(() => { + result.current.updateSearchParams({ panel: 'inspector' }); + }); + + expect(pushSpy).toHaveBeenCalledTimes(1); + expect(result.current.params.panel).toBe('inspector'); + }); + }); + describe('replaceSearchParams', () => { test('replaces all params with new ones (clears unspecified params)', () => { history.replaceState({}, '', '/workflow?panel=run&job=abc&step=5'); From 927c28e8b33e99684615cbfb15720ee6baec83da Mon Sep 17 00:00:00 2001 From: Taylor Downs Date: Fri, 29 May 2026 12:54:58 +0200 Subject: [PATCH 2/4] CL --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54903c47e07..855b97aefaa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ and this project adheres to ### Fixed +- Fix issue where back button must be pressed 3 times to go back once from the + Workflow canvas [#4812](https://github.com/OpenFn/lightning/issues/4812) - Fix `purge_deleted` Oban job crashing when a soft-deleted project has associated OAuth clients. The `project_oauth_clients` join rows are now cleaned up alongside the other project-scoped deletes in From 34ad012bcbc4506c1b33b06e21b702d2679f3e7e Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Wed, 3 Jun 2026 14:55:35 +0100 Subject: [PATCH 3/4] extend no-op guard to replaceSearchParams --- assets/js/react/lib/use-url-state.ts | 5 ++- assets/test/react/lib/use-url-state.test.ts | 34 +++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/assets/js/react/lib/use-url-state.ts b/assets/js/react/lib/use-url-state.ts index 348ced28414..03dc359fdb7 100644 --- a/assets/js/react/lib/use-url-state.ts +++ b/assets/js/react/lib/use-url-state.ts @@ -117,13 +117,16 @@ class URLStore { newParams: Record ) => { const newURL = new URL(window.location.pathname, window.location.origin); - // Only set params with non-null values (clears all existing params) Object.entries(newParams).forEach(([key, value]) => { if (value !== null) { newURL.searchParams.set(key, String(value)); } }); newURL.hash = window.location.hash; + // Skip no-op writes so mount-time normalization doesn't stack + // duplicate browser history entries (a no-op pushState never notifies + // subscribers anyway, due to the guard in updateParams). + if (newURL.href === window.location.href) return; history.pushState({}, '', newURL); }; diff --git a/assets/test/react/lib/use-url-state.test.ts b/assets/test/react/lib/use-url-state.test.ts index 4dc5c68ae06..3191a02c7cd 100644 --- a/assets/test/react/lib/use-url-state.test.ts +++ b/assets/test/react/lib/use-url-state.test.ts @@ -236,6 +236,40 @@ describe('useURLState', () => { }); }); + describe('replaceSearchParams - no-op writes', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + test('does not push a history entry when params are unchanged', () => { + history.replaceState({}, '', '/workflow?panel=run'); + + const { result } = renderHook(() => useURLState()); + const pushSpy = vi.spyOn(history, 'pushState'); + + act(() => { + result.current.replaceSearchParams({ panel: 'run' }); + }); + + expect(pushSpy).not.toHaveBeenCalled(); + expect(window.location.search).toBe('?panel=run'); + }); + + test('still pushes a history entry when a param actually changes', () => { + history.replaceState({}, '', '/workflow?panel=run'); + + const { result } = renderHook(() => useURLState()); + const pushSpy = vi.spyOn(history, 'pushState'); + + act(() => { + result.current.replaceSearchParams({ panel: 'inspector' }); + }); + + expect(pushSpy).toHaveBeenCalledTimes(1); + expect(result.current.params.panel).toBe('inspector'); + }); + }); + describe('replaceSearchParams', () => { test('replaces all params with new ones (clears unspecified params)', () => { history.replaceState({}, '', '/workflow?panel=run&job=abc&step=5'); From 912c43b93c12fd133c3e8a9fe0cb503f21e9ac47 Mon Sep 17 00:00:00 2001 From: Stuart Corbishley Date: Thu, 4 Jun 2026 10:32:07 +0200 Subject: [PATCH 4/4] Extend no-op URL-history guard to updateHash and hash reorder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The no-op guard added in #4813 covered updateSearchParams and replaceSearchParams but not updateHash, and was order-sensitive — a reorder-only param write still stacked a duplicate history entry. Extract a shared pushIfChanged guard (order-insensitive via sorted search params) and a currentURL helper that all three writers seed from, so every history write goes through one correct check. --- assets/js/react/lib/use-url-state.ts | 56 ++++++++------- assets/test/react/lib/use-url-state.test.ts | 76 +++++++++++++++++++++ 2 files changed, 109 insertions(+), 23 deletions(-) diff --git a/assets/js/react/lib/use-url-state.ts b/assets/js/react/lib/use-url-state.ts index 03dc359fdb7..f088d47f538 100644 --- a/assets/js/react/lib/use-url-state.ts +++ b/assets/js/react/lib/use-url-state.ts @@ -82,6 +82,29 @@ class URLStore { */ getSnapshot = () => this.state; + // A fresh, mutable copy of the current URL. Writers seed from this and + // override only the dimension they own (search or hash); everything else + // carries over untouched. + private currentURL = () => new URL(window.location.href); + + private urlsAreEquivalent = (a: URL, b: URL) => { + if (a.pathname !== b.pathname || a.hash !== b.hash) return false; + const ap = new URLSearchParams(a.search); + ap.sort(); + const bp = new URLSearchParams(b.search); + bp.sort(); + return ap.toString() === bp.toString(); + }; + + // Skip no-op writes so mount-time normalization doesn't stack duplicate + // browser history entries (a no-op pushState never notifies subscribers + // anyway, due to the guard in updateParams). Param order is ignored so a + // reorder-only write is also treated as a no-op. + private pushIfChanged = (newURL: URL) => { + if (this.urlsAreEquivalent(newURL, this.currentURL())) return; + history.pushState({}, '', newURL); + }; + /** * Update URL search params (merges with existing params). * Accepts strings, numbers, booleans; null removes param. @@ -89,24 +112,17 @@ class URLStore { updateSearchParams = ( updates: Record ) => { - const newParams = new URLSearchParams(window.location.search); + const newURL = this.currentURL(); Object.entries(updates).forEach(([key, value]) => { if (value === null) { - newParams.delete(key); + newURL.searchParams.delete(key); } else { - newParams.set(key, String(value)); + newURL.searchParams.set(key, String(value)); } }); - const newURL = new URL(window.location.pathname, window.location.origin); - newURL.search = newParams.toString(); - newURL.hash = window.location.hash; - // Skip no-op writes so mount-time normalization doesn't stack - // duplicate browser history entries (a no-op pushState never notifies - // subscribers anyway, due to the guard in updateParams). - if (newURL.href === window.location.href) return; - history.pushState({}, '', newURL); + this.pushIfChanged(newURL); }; /** @@ -116,18 +132,14 @@ class URLStore { replaceSearchParams = ( newParams: Record ) => { - const newURL = new URL(window.location.pathname, window.location.origin); + const newURL = this.currentURL(); + newURL.search = ''; Object.entries(newParams).forEach(([key, value]) => { if (value !== null) { newURL.searchParams.set(key, String(value)); } }); - newURL.hash = window.location.hash; - // Skip no-op writes so mount-time normalization doesn't stack - // duplicate browser history entries (a no-op pushState never notifies - // subscribers anyway, due to the guard in updateParams). - if (newURL.href === window.location.href) return; - history.pushState({}, '', newURL); + this.pushIfChanged(newURL); }; /** @@ -135,11 +147,9 @@ class URLStore { * Pass null to remove hash. */ updateHash = (hash: string | null) => { - const newURL = - window.location.pathname + - window.location.search + - (hash ? `#${hash}` : ''); - history.pushState({}, '', newURL); + const newURL = this.currentURL(); + newURL.hash = hash ? `#${hash}` : ''; + this.pushIfChanged(newURL); }; } diff --git a/assets/test/react/lib/use-url-state.test.ts b/assets/test/react/lib/use-url-state.test.ts index 3191a02c7cd..da1ee6a316c 100644 --- a/assets/test/react/lib/use-url-state.test.ts +++ b/assets/test/react/lib/use-url-state.test.ts @@ -268,6 +268,82 @@ describe('useURLState', () => { expect(pushSpy).toHaveBeenCalledTimes(1); expect(result.current.params.panel).toBe('inspector'); }); + + test('does not push when params are reordered but unchanged', () => { + history.replaceState({}, '', '/workflow?job=abc&panel=run'); + + const { result } = renderHook(() => useURLState()); + const pushSpy = vi.spyOn(history, 'pushState'); + + act(() => { + // Same params, different object-key order — semantically identical URL + result.current.replaceSearchParams({ panel: 'run', job: 'abc' }); + }); + + expect(pushSpy).not.toHaveBeenCalled(); + expect(window.location.search).toBe('?job=abc&panel=run'); + }); + }); + + describe('updateHash - no-op writes', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + test('does not push a history entry when hash is unchanged', () => { + history.replaceState({}, '', '/workflow#step-2'); + + const { result } = renderHook(() => useURLState()); + const pushSpy = vi.spyOn(history, 'pushState'); + + act(() => { + result.current.updateHash('step-2'); + }); + + expect(pushSpy).not.toHaveBeenCalled(); + expect(window.location.hash).toBe('#step-2'); + }); + + test('does not push when clearing an already-absent hash', () => { + history.replaceState({}, '', '/workflow'); + + const { result } = renderHook(() => useURLState()); + const pushSpy = vi.spyOn(history, 'pushState'); + + act(() => { + result.current.updateHash(null); + }); + + expect(pushSpy).not.toHaveBeenCalled(); + }); + + test('still pushes when adding a hash', () => { + history.replaceState({}, '', '/workflow'); + + const { result } = renderHook(() => useURLState()); + const pushSpy = vi.spyOn(history, 'pushState'); + + act(() => { + result.current.updateHash('step-5'); + }); + + expect(pushSpy).toHaveBeenCalledTimes(1); + expect(window.location.hash).toBe('#step-5'); + }); + + test('still pushes when removing an existing hash', () => { + history.replaceState({}, '', '/workflow#a'); + + const { result } = renderHook(() => useURLState()); + const pushSpy = vi.spyOn(history, 'pushState'); + + act(() => { + result.current.updateHash(null); + }); + + expect(pushSpy).toHaveBeenCalledTimes(1); + expect(window.location.hash).toBe(''); + }); }); describe('replaceSearchParams', () => {