Skip to content

Guard EditableTextField against overlapping saves #425

Open

Description

@nsdeschenes

Problem

EditableTextField.save() can invoke onSave again while a previous save is pending. The Save button is disabled during saving, but keyboard submissions bypass that protection.

This behavior predates the StyleX migration in #408.

Reproduction

Verified in a browser with the actual component and a deferred save callback:

  1. Render the editor with an initial value and an onSave callback that records its payload and returns an unresolved promise.
  2. Edit the value and press Enter in the single-line input.
  3. Keep the first save pending and press Enter again.

The callback is invoked twice before either save completes. Changing the draft after an optimistic parent update also allows a second save while the first is pending.

Expected behavior is at most one active save per editor, regardless of keyboard or button activation.

Optimistic update nuance

The incident screen updates the cached value optimistically. Repeating Enter with an unchanged draft can instead take the draftValue === value early return and close the editor while its save is pending. A later failure can then lose its visible error message. Repeated Enter does not always cause another request.

Cause and suggested fix

The save callback sets isSaving for presentation but does not check for an active save.

Use a synchronous per-instance guard at the beginning of save(), before the unchanged-value early return. Acquire it before invoking onSave and release it in finally. Keep isSaving for the loading/disabled UI. A React state check alone does not exclude multiple calls through the same render closure before React commits.

Regression coverage should include same-turn keyboard/button submissions, later keyboard submissions while pending, optimistic value changes keeping the editor open, and successful retry after rejection or a synchronous callback throw.

Related issue #424 tracks shortcut routing.

History

Identified in this Warden comment. The same unguarded save exists in the pre-migration component and dates to the original component addition.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions