Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

could we add leading/trailing edge options for debounce? #2380

Open
ensconced opened this issue May 31, 2023 · 3 comments · May be fixed by #2382
Open

could we add leading/trailing edge options for debounce? #2380

ensconced opened this issue May 31, 2023 · 3 comments · May be fixed by #2382

Comments

@ensconced
Copy link

ensconced commented May 31, 2023

This is more of a feature request than a bug. If you think this feature might make sense then maybe I will raise a PR for it.

Could we add options for debounce to configure whether the saga is called on the leading edge and/or the trailing edge? (Compare to the lodash debounce function).

This would be a very useful feature because by setting leading: true I can ensure I respond immediately to user actions without having to wait for the debounce period to expire.

Perhaps the first delayLength parameter could either be number, or an options object like { delayLength: number; leading?: boolean; trailing?: boolean }?

@neurosnap
Copy link
Member

Hi! Thanks so much for reaching out.

First and foremost: redux-saga is in a stable state and as such backwards compatibility is our primary directive. If you can implement the ability to change debounce -- and throttle -- in a non-breaking way, I'd be happy to review and approve.

I'm also happy to answer any questions you have.

@ensconced
Copy link
Author

ensconced commented Jun 6, 2023

I made a start on this. It seems straightforward enough for debounce - my initial attempt at that is here (work in progress) but throttle is a little trickier.

I think the current implementation of throttle would correspond to the leading: true, trailing: true case. For the leading, true, trailing: false case, I think it's as simple as just using buffers.none() instead of buffers.sliding(1)). And leading: false, trailing: false should be simple too (since it basically does nothing).

However, I'm not sure how to approach the leading: false, trailing: true case for throttle.
I wanted to use an fsm like this:

fsmIterator(
  {
    q1() {
      const yActionChannel = { done: false, value: actionChannel(patternOrChannel, buffers.sliding(1)) }
      return { nextState: 'q2', effect: yActionChannel, stateUpdater: setChannel }
    },
    q2() {
      return { nextState: 'q3', effect: yTake(), stateUpdater: setAction }
    },
    q3() {
      return { nextState: 'q4', effect: yDelay }
    },
    q4() {
      // If another action has been received since the leading edge,
      // use that for the trailing edge.
      return channel.isEmpty()
        ? { nextState: 'q5', effect: yTake, stateUpdater: setAction }
        : { nextState: 'q5', effect: yNoop }
    },
    q5() {
      return { nextState: 'q2', effect: yFork(action) }
    },
  },
  needsChannel ? 'q1' : 'q2',
  `throttle(${safeName(patternOrChannel)}, ${worker.name})`,
)

...but channel.isEmpty() is a non-existent method. I'm not sure how to achieve this without some way of checking whether the channel is empty.

@neurosnap
Copy link
Member

Hi! You are welcome to just focus on debounce for now if that makes things simpler for you.

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

Successfully merging a pull request may close this issue.

2 participants