fix: Fix invalid blocker state transition in UnsavedChangesProvider (#1184)

[karilint]

Fix #1184: Time Unit error caused by invalid blocker state transition.

Root Cause:
setDirty(false) was called before blocker.proceed?.() in UnsavedChangesProvider, which triggered a re-render that changed the blocker state before proceed was called. This caused React Router's internal state machine to receive a proceed call when the blocker was no longer in the 'blocked' state.

Error:

Invalid blocker state transition: unblocked -> proceeding

Fix:

• Call blocker.proceed?.() before setDirty(false) in handleConfirm
• Add additional state check in isSamePathNavigation useEffect to ensure blocker is still 'blocked' before calling proceed()

Testing:

• Manual test: Edit Time Unit, change upper bound, press return to table, unsaved changes prompt appears, press leave page - error should not occur

[karilint]

@copilot Please review this pull request.

[Copilot]

@copilot Please review this pull request.

Here's my review of this PR:

Summary

The fix correctly addresses the root cause of the Invalid blocker state transition: unblocked -> proceeding error.

Change 1 — handleConfirm: swap order of proceed and setDirty(false) ✅

Before:

setDirty(false)       // triggers re-render → blocker transitions to 'unblocked'
blocker.proceed?.()   // called when blocker is already 'unblocked' → error

After:

blocker.proceed?.()   // called while blocker is still 'blocked' ✓
setDirty(false)       // safe; both updates batch in React 18

This is the correct fix. The previous order caused React to re-render synchronously from setDirty(false), which re-evaluated shouldBlockNavigation (returning false since isDirty was now false), causing React Router to move the blocker out of 'blocked' state before proceed() had been invoked.

Change 2 — isSamePathNavigation useEffect: add blocker.state === 'blocked' guard ⚠️ Redundant but harmless

if (isSamePathNavigation && proceed && blocker.state === 'blocked') {

isSamePathNavigation is defined as:

const isSamePathNavigation = blocker.state === 'blocked' && blocker.location?.pathname === window.location.pathname

The added blocker.state === 'blocked' check is already implied by isSamePathNavigation === true, so it is redundant. Both blocker and isSamePathNavigation are listed as effect dependencies, so they're always in sync when the effect runs. The guard doesn't cause any harm, but it's worth noting the redundancy.

Test coverage

The existing tests in UnsavedChangesProvider.test.tsx cover:

• Showing the dialog and calling proceed on confirm ✓
• Auto-proceeding for same-path navigation ✓
• Not showing dialog when not dirty ✓

The tests pass with the current fix, though they don't explicitly assert the ordering of proceed vs setDirty. That level of assertion would require mocking state updates, which may be over-engineering for this case.

Overall

The fix is correct and minimal. The primary change (swapping the order in handleConfirm) properly addresses the race condition. The secondary guard is redundant but won't introduce regressions.

[karilint]

@copilot Please review this pull request.

[Copilot]

@copilot Please review this pull request.

The PR is in good shape. Here's the updated review reflecting both commits:

Current state of changes

Commit 1 (4ae003a) — swap proceed / setDirty order in handleConfirm ✅
Commit 2 (7e732d9) — remove the redundant blocker.state === 'blocked' guard from the isSamePathNavigation useEffect ✅

The code is now correct and clean:

// handleConfirm — correct order
blocker.proceed?.()   // called while blocker is still 'blocked'
setDirty(false)       // safe to update state after proceed

// isSamePathNavigation useEffect — clean, no redundancy
if (isSamePathNavigation && proceed) {
  proceed()
}

No issues found. The fix is minimal and correctly addresses the root cause of the Invalid blocker state transition: unblocked -> proceeding error.