Skip to content

fix: attach form submit listener before paint to prevent reload flake - #61

Merged
Archmonger merged 1 commit into
mainfrom
fix-form-no-reload-flake
Sep 21, 2026
Merged

Archmonger merged 1 commit into
mainfrom
fix-form-no-reload-flake

Conversation

@Archmonger

Copy link
Copy Markdown
Contributor

Description

test_form_no_page_reload was flaky in CI (e.g. run 35571256796, python-source 3.13), failing with:

assert render_count.current == 1
E   assert 2 == 1

Root cause

The Form component (from #58) wires its submit event listener inside React.useEffect, which runs after the browser paints. That leaves a window — widest on a slow/loaded CI machine — after the form is rendered but before the submit listener is attached.

When a submit fires during that window, the listener's preventDefault() never runs, so the browser performs its native GET form submission → a full page reload. ReactPy remounts the app and re-runs the route component, so the server-side Ref render counter reaches 2 (and the client-side router/scroll store is wiped). This is the same class of defect as the Link preventDefault race that was fixed for scroll restoration in #59.

Fix

Attach the submit listener in React.useLayoutEffect instead of React.useEffect. useLayoutEffect runs synchronously before the browser paints, so the preventDefault handler is always in place by the time the form is visible and interactive — eliminating the fall-through-to-native-submit window entirely.

React is preact/compat from @reactpy/client, which provides useLayoutEffect.

Verification

Before the fix, the form-no-reload test reproduced render_count == 2 under load. After the fix:

  • 15/15 CI-style hatch test --headless runs
  • 27/27 genuine runs under taskset -c 0,1 -n 6 oversubscription (the only 3 "failures" were unrelated bun install EEXIST harness races from concurrent xdist workers)
  • Full suite 30/30 passed across 3 runs
  • Prettier/ESLint and pyright clean

Checklist

Please update this checklist as you complete each item:

  • Tests have been developed for bug fixes or new functionality.
  • The changelog has been updated, if necessary.
  • Documentation has been updated, if necessary.
  • GitHub Issues closed by this PR have been linked.

By submitting this pull request I agree that all contributions comply with this project's open source license(s).

test_form_no_page_reload could fail with render_count == 2 in CI. The Form
component attached its submit listener in React.useEffect, which runs *after*
the browser paints. That leaves a window after the form is rendered but before
the listener is wired, during which a submit falls through to the browser's
native GET submission - a full page reload that remounts the app and re-runs
the route component, bumping the render counter.

Attach the listener in useLayoutEffect instead, which runs synchronously
before paint, so the preventDefault handler is always in place by the time the
form is visible and interactive. This mirrors the Link preventDefault race
addressed for scroll restoration.
@Archmonger
Archmonger marked this pull request as ready for review September 21, 2026 08:06
@Archmonger
Archmonger requested a review from a team as a code owner September 21, 2026 08:06
@Archmonger
Archmonger merged commit ca8a75f into main Sep 21, 2026
9 checks passed
@Archmonger
Archmonger deleted the fix-form-no-reload-flake branch September 21, 2026 08:25
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.

1 participant