Skip to content

fix: no-loading-flag-reset-outside-finally false positive with trailing reset - #1595

Draft
skoshx wants to merge 1 commit into
mainfrom
cursor/triage-1593-63ad
Draft

fix: no-loading-flag-reset-outside-finally false positive with trailing reset#1595
skoshx wants to merge 1 commit into
mainfrom
cursor/triage-1593-63ad

Conversation

@skoshx

@skoshx skoshx commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #1593 - False positive where no-loading-flag-reset-outside-finally incorrectly flagged trailing resets after non-rethrowing catch blocks when the catch contained user code calls like toast.show().

Root Cause

The rule conservatively assumed all potentially-throwing calls in catch handlers would prevent trailing resets from running. This created false positives for user code that doesn't actually throw.

The pattern from the issue:

try {
  const res = await fetch("/api/upload");
  // ... 
} catch {
  toast.show({ variant: "danger", label: "Upload error", duration: 3500 });
}
setIsUploading(false); // <- incorrectly flagged

This is safe: if the catch doesn't rethrow, the trailing reset will run on both success and rejection paths.

The Fix

Modified subtreeHasAbruptSynchronousOperation to use lenient mode when checking catch handlers with trailing resets. Lenient mode only flags:

  1. Explicit throw/return statements - definitely prevent trailing reset
  2. Calls to local functions proven to always throw - e.g. const rethrow = () => { throw ... }; rethrow();
  3. Built-in calls that might throw - e.g. JSON.parse(), Math.round(1n), console[method]()

User code calls on non-built-in objects (like toast.show()) are now assumed safe.

Why This Matters

This resolves the conflict with React Compiler, which cannot handle try/finally and requires the catch-without-rethrow + trailing-reset pattern. From the issue:

React Compiler cannot currently lower try/finally (or try without catch). So the two rules are in direct conflict:

  • no-loading-flag-reset-outside-finally → wants finally
  • react-hooks-js/todo → forbids finally

Testing

  • ✅ All existing tests pass
  • ✅ Added regression test for the reported pattern
  • ✅ Verified that true positives are still caught (local throwing functions, built-ins)

Parity

Running rde parity to validate no cross-repo regressions. Results will be added as a comment.

Closes #1593

Open in Web Open in Cursor 

…ng reset

The rule incorrectly flagged trailing resets after non-rethrowing catch
blocks when the catch contained user code calls like toast.show().

Root cause: The rule conservatively assumed all potentially-throwing calls
in catch handlers would prevent trailing resets from running. This created
false positives for user code that doesn't actually throw.

Fix: When checking if a catch handler can bypass a trailing reset, use
lenient mode that only flags:
- Explicit throw/return statements
- Calls to local functions proven to always throw
- Built-in calls that might throw (JSON.parse, Math.round, etc.)

User code calls on non-built-in objects are now assumed safe, resolving
the conflict with React Compiler which cannot handle try/finally.

Fixes #1593

Co-authored-by: Skosh <skoshx@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

False Positive: react-doctor/no-loading-flag-reset-outside-finally

2 participants