don't write no-ops as new URL history items

CHANGELOG.md

@@ -27,6 +27,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)
 - Reduce `run:log` channel timeouts under heavy log volume by moving `log_lines`
   search indexing off the insert path. The full-text search vector is now
   backfilled by a background worker rather than computed synchronously on every
@@ -38,7 +40,8 @@ and this project adheres to
   the whole run. The search vector is now built off the insert path by a
   background `Lightning.Invocation.DataclipSearchVectorWorker` (sharing the
   `search_indexing` queue with the log-lines worker), making dataclip search
-  eventually consistent. [#4800](https://github.com/OpenFn/lightning/issues/4800)
+  eventually consistent.
+  [#4800](https://github.com/OpenFn/lightning/issues/4800)
 - Channel join crashes when multiple users open the same workflow concurrently
   [#4802](https://github.com/OpenFn/lightning/issues/4802)
 - Fix `purge_deleted` Oban job crashing when a soft-deleted project has

assets/js/react/lib/use-url-state.ts

@@ -82,27 +82,47 @@ 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.
    */
   updateSearchParams = (
     updates: Record<string, string | number | boolean | null>
   ) => {
-    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;
-    history.pushState({}, '', newURL);
+    this.pushIfChanged(newURL);
   };
 
   /**
@@ -112,27 +132,24 @@ class URLStore {
   replaceSearchParams = (
     newParams: Record<string, string | number | boolean | null>
   ) => {
-    const newURL = new URL(window.location.pathname, window.location.origin);
-    // Only set params with non-null values (clears all existing params)
+    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;
-    history.pushState({}, '', newURL);
+    this.pushIfChanged(newURL);
   };
 
   /**
    * Update the URL hash fragment.
    * 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);
   };
 }
 

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,151 @@ 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 - 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');
+    });
+
+    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', () => {
     test('replaces all params with new ones (clears unspecified params)', () => {
       history.replaceState({}, '', '/workflow?panel=run&job=abc&step=5');