Skip to content

fix: createChannelFromSelector emits a transition twice when the worker dispatches synchronously (baseline recorded after emit) #21

Description

@panghy

Sibling of #20 (same function, independent bug; reproduces on stock 0.2.7 with or without the #20 buffer fix).

Summary

createChannelFromSelector (src/utils/sagas/selector-channel-effects.ts) updates its comparison baseline after calling the emitter:

const payload = cachedSelector(reduxStore.getState(), ...args);
if (shallowEqual(prevValue, payload)) return;
emitter({ payload, prevPayload: prevValue });
prevValue = payload;   // too late — see below

emitter() is synchronous: with a taker attached it resumes the take*FromSelector loop, which forks the worker, and the worker's queued put is flushed by the redux-saga scheduler before emitter() returns. If that dispatch is a reducer no-op, the store still notifies subscribers, emitCurrentValue re-enters while prevValue is still the old value, shallowEqual(old, payload) is false again, and the same transition is emitted a second time (payload B, prevPayload A, twice) for a single store update.

If the worker dispatches a real state change instead, the nested emission carries the wrong prevPayload (A → C instead of B → C).

Repro

Real Redux createStore + saga middleware, takeEveryFromSelector(selectValue, worker):

  1. Initial state { value: 'A' }, selector state => state.value.
  2. Worker records [prevPayload, payload], then yield put({ type: 'noop' }) (reducer returns the same state).
  3. store.dispatch(set('B')).

Observed: worker called twice with ['A', 'B']. Expected: once.

Variant: worker puts set('C') when it sees B → observed [['A','B'], ['A','C']], expected [['A','B'], ['B','C']].

Suggested fix

Record the baseline before emitting, keeping prevPayload semantics:

const prevPayload = prevValue;
prevValue = payload;
emitter({ payload, prevPayload });

The selector still throws before the assignment, so reportRuntimeError handling and the retained old baseline on selector failure are unchanged.

Downstream tracking: intent-hq/intent#5040 (interim pnpm patch in cloudlands-fe, alongside the #20 patch).

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions