elm-datepicker icon indicating copy to clipboard operation
elm-datepicker copied to clipboard

Why does update return a Cmd Msg?

Open ahstro opened this issue 6 years ago • 4 comments

Looking at the update function, it returns a three tuple containing, among other things, a Cmd Msg. However, looking at all branches of the update's case statement, neither one returns anything other than Cmd.none for that Msg. Is this simply left in here to prevent a major version bump or is it there for another reason?

ahstro avatar Feb 20 '18 09:02 ahstro

I believe it's just that it used to return commands that were used to, among other things, need/retrieve the current date. It's probably cruft that just wasn't removed along with that feature. That is, I'm certainly not aware of any intentional retention of the return type just to pad against changes (although that's not a bad idea, Elm is so major-version happy it feels like it wouldn't buy us much 😄 ).

Thoughts? We could either:

  • update docs to say "lol sorry ignore"
  • update the return type

The latter is more correct but bumps us without an actual functionality change; perhaps we could bundle it with another PR as a release.

bbqbaron avatar Feb 26 '18 23:02 bbqbaron

Ha! Actually, #40 was exactly this question and I apparently thought it was wise to retain the Cmd at the time (in my defense, this was like 10 months ago!). I could just add an FAQ, maybe?

bbqbaron avatar Feb 26 '18 23:02 bbqbaron

Adding an FAQ would probably be the easiest solution, but I'd like to make a case for a major version bump (with a clear changelog or release note, so people know it's easy to upgrade).

Besides the obvious argument "it doesn't do anything", another reason to remove it would be because it introduces debugging complexity where there is no need for it. Say, for example, I'm having some weird side effect issue; even though we know that elm-datepicker isn't involved, simply seeing that it handles Cmds introduces the possibility that it could be. In my opinion, one of the strongest features of the type system is to see what certain chunks of code can feasably do without having to read them, and leaving it in sort of goes against that.

But, it's ultimately up to you and what you think works best for the lib. I'm loving it regardless :smile:

ahstro avatar Feb 27 '18 10:02 ahstro

Effects cruft does seem kind of perverse (you can tell that I don't have a strong opinion on this because of how easy I am to convince!). My last opinion being a year old, I have no problem revisiting. #82 is about to cause a major bump, so we could bundle that change with it for sure.

Fortunately, the fact that 0.19 will retire this repo means we're a liiiiittle less on the hook for long-term futureproofing. (Heck, maybe we should go totally gonzo and add bizarro experimental features 🤔 )

bbqbaron avatar Mar 04 '18 16:03 bbqbaron