hayleigh-dot-dev / elm-limiter

Throttling and debouncing for messages and values.
MIT License
4 stars 3 forks source link

Requiring consumers to store a function on their model may be ill-advised #2

Open derrickbeining opened 3 years ago

derrickbeining commented 3 years ago

I really like the API for your library. We were about to use it in one of our projects where I work until we realized that the signatures of debounce and throttle required us to store a reference to a function on the model, which is something that is generally advised against. To quote @evancz himself

One of the core rules of The Elm Architecture is never put functions in your Model or Msg types. ^1

Some further reading/discussion on the topic:

I just wanted to bring this issue to your attention in case it is something you were unaware of while implementing this library. I think modifying the implementation such that the model doesn't store functions would make this package more generally usable for everyone.

hayleigh-dot-dev commented 3 years ago

I can look into changing things around at some point (or you can and open a PR 👀), IME the argument is a philosophical one more than a practical one.

The two major points are:

Personally, after 3 years of Elm I have not once used that feature of the debugger (and very, very rarely use the time travel debugger because it was broken for the majority of 0.19.0 anyway), and I'd say you're probably doing something weird if you need to equality check your limiters.

If it's a dealbreaker for you that's a bit of a bummer, as I say I'm open to PRs if you want to work around it, but my own experience suggests this isn't a big deal.