Skip to content

Fix flicker bug#471

Merged
dfltr merged 5 commits intomainfrom
flicker-bug
Mar 13, 2026
Merged

Fix flicker bug#471
dfltr merged 5 commits intomainfrom
flicker-bug

Conversation

@dfltr
Copy link
Contributor

@dfltr dfltr commented Mar 12, 2026

Slack context: https://frigade.slack.com/archives/C055JES0D8D/p1772854904216229

Hypothesis is that Simplify's high traffic caused latency that revealed a few existing race conditions. This patch is comprised of three fixes that all address Flow visibility races in the JS API.

  1. Don't refresh local flow state when awaiting serial COMPLETED_* API calls. The response to COMPLETED_STEP could conflict with the response from COMPLETED_FLOW.
  2. Don't call step.start() in the body of the Flow component, otherwise it can fire erroneous starts during unrelated re-renders.
  3. Similar to the first item, skip refreshStateFromAPI calls when pendingRequests >0, to avoid having outdated server state written back over newer (more correct) optimistic local state.

dfltr and others added 5 commits March 12, 2026 15:20
When completing the last step, internalComplete sends two sequential API
calls: COMPLETED_STEP then COMPLETED_FLOW. Previously, the COMPLETED_STEP
response would trigger refreshStateFromAPI, which could revert the
optimistic visible=false state (since the server hadn't received
COMPLETED_FLOW yet). This caused the flow to briefly reappear (flicker).

Add a skipRefresh parameter to sendFlowStateToAPI and pass it when
the COMPLETED_STEP call is followed by COMPLETED_FLOW, so only the
final response refreshes state.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
step.start() was called as a side-effect during render, which meant
every intermediate re-render (including during the flicker window)
would fire a new STARTED_STEP API call. This created competing requests
that could revert dismiss/complete state.

Moving it to a useEffect ensures it only fires after React commits,
and the dependency array prevents redundant calls.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… requests

When refreshStateFromAPI processes eligible flows, it now skips updating
any flow that has in-flight API requests (pendingRequests > 0). This
prevents stale data from generic polls, visibility-change refreshes, or
other flows' API responses from reverting optimistic state for a flow
that has a pending skip/complete/start action.

sendFlowStateToAPI already decrements pendingRequests before calling
refreshStateFromAPI, so the acting flow's own count will be 0 and its
authoritative response will still be applied.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update the three flicker reproduction tests to verify the fixes work:

1. Completing last step now keeps flow invisible throughout (no
   visibility reverting between COMPLETED_STEP and COMPLETED_FLOW)
2. Optimistic completed state is preserved after COMPLETED_STEP,
   preventing the React autoStart condition from re-firing step.start()
3. Stale state refreshes no longer overwrite optimistic skip state
   for flows with pending API requests

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dfltr dfltr requested a review from christianmat March 12, 2026 23:48
Comment on lines +616 to +620
if (
(frigadeGlobalState[globalStateKey].pendingRequests[statefulFlow.flowSlug] ?? 0) > 0
) {
return
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non blicking - but why is this formatted like this lol

Copy link

@eltoncrego eltoncrego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! I didnt read the tests

Copy link
Contributor

@christianmat christianmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have a few automated storybook e2e tests in here - minus the flaky collection ones, let's verify the ones pass?

@dfltr dfltr merged commit 785a576 into main Mar 13, 2026
1 of 2 checks passed
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.

3 participants