Clarify semantics of set/modState

[tgiddings]

Was: BackendScope's state callback returns state from when callback is constructed rather than from when it's executed

BackendScope has a state function that returns a callback. However, it returns the state as of callback construction rather than callback execution. This breaks the callback's composability. e.g.

case class Backend($: BackendScope[Unit, State]) {
  def submit = $.modState(s => if(s.username=="") s.copy(usernameMissing = true) else s.copy(usernameMissing = false)) >> 
    $.modState(s => if(s.password=="") s.copy(passwordMissing = true) else s.copy(passwordMissing = false)) >>
    $.state.map{
      state =>
        if(!state.usernameMissing && !state.passwordMissing) dom.window.alert("Submitted")
    }

will popup "Submitted" even if username and password are "" if usernameMissing and passwordMissing hadn't already been set. The effects of the $.modstates are not seen by $.state (but $.modstates effects are seen by eachother, meaning this can be worked around by replacing $.state with $.modstate)

[japgolly]

You've hit upon semantics of React itself there. set/modState is async and they make no guarantee about when they'll apply the state change. Callbacks involvement just wraps calls to underlying React methods.

If I were you I'd separate the pure state logic from the Callback context. So either one big modState{ withAllLogicInside } or extract an S => S.

Another option is to use state monads (live demo). It would be a much closer fit to the style of coding above.

[tgiddings]

To clarify, does that mean that modState can return and move on to the next callback before react has modified the state? If so, that fact should probably be in large, bold letters in the documentation. Callback has a monadic and composable interface. There's nothing to inform the user that this doesn't work for the Callbacks from BackendScope.

[tgiddings]

Also, the state monads do indeed work well for my use case and behave in the way I expected. The reason I didn't do modState{ withAllLogicInside } is that that would lead to a lot of duplicate code that I would need to keep in sync.

[japgolly]

To clarify, does that mean that modState can return and move on to the next callback before react has modified the state? If so, that fact should probably be in large, bold letters in the documentation. Callback has a monadic and composable interface. There's nothing to inform the user that this doesn't work for the Callbacks from BackendScope.

Yes, that's exactly right. If you want to instruct React to do extra stuff after the state change has been applied, you need to pass it as the second argument to set/modState. This stuff is all in the React JS doc but it doesn't hurt to have it on our side too; feel free to PR.

Also, the state monads do indeed work well for my use case and behave in the way I expected. The reason I didn't do modState{ withAllLogicInside } is that that would lead to a lot of duplicate code that I would need to keep in sync.

Good to hear

[tgiddings]

Yes, that's exactly right. If you want to instruct React to do extra stuff after the state change has been applied, you need to pass it as the second argument to set/modState.

Would it be possible to have BackendScope return a specialized CallbackTo class that takes advantage of the callback argument of React's setState so that they compose correctly? The new class could have a list where the nodes of the list have a state transformation and a sequence of normal callbacks. The list then gets transformed to (pseudocode)

setState(node1transformation,{
    node1callback1
    node1callback2
    ....
    setState(node2transformation,{
        node2callback1
        node2callback2
        ....
        setState(....)
    })
})

The only big difficulty I see is how to make it work when the first (since >> is left-binding) callback is a normal callback. Since CallbackTo is currently final, it could be made into a sealed class or trait instead of an open one. Since it's sealed, it probably wouldn't be to big of a deal if >> and co. pattern match on the type of runNext and return runNext << this or similar if the left-hand side is a normal callback and the right-hand side is the specialized one for state. I'll try to make a gist later today that goes into more detail on a possible implementation.

This stuff is all in the React JS doc

It's not always clear how much a language-adapting library adheres to the semantics of the original library vs finds a way to make an interface that is consistent with the target language or other parts of the adapter library. To wit: I would consider these two callbacks to be inconsistent with the semantics of the CallbackTo class. $.modState(...) >> doX is "modify the state, then do x," but this isn't the case formodState and co.

[japgolly]

This is a really interesting direction of thought and if I had the time/freedom to explore it with you, I'd love to, but with my Big Busy Maintainer's Hat on, I have to say no, we won't be implementing this suggestion. My reasons are as follows:

• library stability and affect on end-users ← huge consideration given existing userbase and proximity to v1.0 which just went through a huge amount of disruption
• modState composition should work as expected. Callbacks to state/setState need to be aware of the semantics of React itself.
• We already have state monads as a synchronous alternative
• Semantics of Callback aren't flawed or erroneous. They are simply "here-is-code-to-execute-in-some-callback". $.modState(...) >> doX = "when/if you call back, schedule a state modification then do X"
• Extension/conflation of Callback itself to support magic to workaround React's semantics horrifies me as a concept. They should be separate, hopefully-simple concepts and composed as such. (Sorry if that's pedantic cos the actual implementation wasn't your main point.)

[tgiddings]

$.modState(...) >> doX = "when/if you call back, schedule a state modification then do X"

I'm sorry, but this is definitively not the impression given by that code snippet. Perhaps a less extreme method of dealing with that is to rename modState to something like scheduleStateModification or eventuallyModState (Hopefully something shorter; I’m just using the names to prove a point). This signals to the user that the modification of the state will not happen over the course of the callback's execution, whereas modState is written in the imperative with respect to "modify." Ie. "Right now, I will modify the state" as opposed to the actual behaviour "Right now, I will tell someone to make sure the state is modified eventually." "schedule...." in a method name makes the latter the expected behaviour.

Semantics of Callback aren't flawed or erroneous

I'm not saying Callback's semantics are problematic. I'm saying modState, depending on interpretation, either doesn't adhere to Callback's semantics or does something different than its name says it does (My CallbackTo-modifying suggestion takes the former interpretation; my renaming suggestion the latter). modState violates the principle of least surprise (as evidenced by the fact that I completely misatributed the effects of its semantics to a non-existent bug in state)

I agree that my suggestion to modify CallbackTo shouldn't be used; It wasn't the most though-out fix and was very heavy-handed and inelegant. However, I don't think the current status of modState is okay either. It's unclear from the name that it adheres to the semantics of react's setState (and I think setState is itself poorly named for identical reasons to why I think modState has a misleading name). As I mentioned before, for a library like this (adapting a library between two languages and between two paradigms), it's not always clear what adheres to the semantics of the original library and what has been papered over to match the semantics of the target language. This is especially true when the name are different (modState vs setState).

[japgolly]

I feel that the only point your proving is that ambiguity and room for misinterpretation exists. That's true here and every where else too. I can appreciate that some things are unclear and some assumptions won't hold for newcomers to the library -- I get that it must've caused you some frustration. There is value in working to reduce ambiguity and increase clarity but they need to be balanced against other important values too.

The only issue I see here is that {set,mod}State don't communicate that they are async. My rebuttal is that setState is named such in React and it's React semantics (the doc even says React knowledge is a prerequisite). Your rebuttal is that it's still unclear and could be made clearer by something like eventuallyModState. Unfortunately, I disagree: users of all libraries are expected to accrue basic implicit knowledge to the library, all APIs have implicit information (even uncreative Java APIs that think they don't), keeping the setState name in sync with the React API is important for clarity, and finally I want to minimise API disruption for a long while - the v1.0 migration will huge.

Therefore, do you have any other suggestions? Given aforementioned constraints, I think updating the ScalaDoc and user guides could be a valuable way to address what we both agree with, and I'd be glad to review a PR.

[tgiddings]

I feel that the only point your proving is that ambiguity and room for misinterpretation exists.

I don't think it's splitting hairs to differentiate between "room for ambiguity" and "misleading," and think this is firmly the latter, but API stability is a compelling reason not to clarify the name, and a note in the docs will probably be sufficient, especially since the docs for this are in the getting-started series of docs.

Unfortunately, I disagree: users of all libraries are expected to accrue basic implicit knowledge to the library, all APIs have implicit information (even uncreative Java APIs that think they don't)

I'm not sure I understand what you're saying here. Accrue from where? The only source I can think of is experience with/ the documentation of React, and I've twice tried to explain why that knowledge might not transfer the way you've described. Perhaps I'm overestimating the likelihood of an experience like mine, though.

do you have any other suggestions

The only categories of change I know of to fix an issue are to change the behaviour, change the name, and change the documentation, and the first two categories have been dismissed as wholes. Adding a note to the documentation is the only valid solution left.

The only issue I see here is that {set,mod}State don't communicate that they are async.

Probably the biggest reason I find modState's semantics confusing is that there are two separate layers of asynchrony whilst React has one. The extra indirection was very surprising; React's setState will, eventually, perform a state change, then execute a callback. modState will, eventually, tell react to, eventually, make a state change, and after it's done telling react to eventually make a state change, execute the >>-ed callback.

(Also, what should we retitle this issue to? My original title is horribly misrepresentative of the issue and what is being discussed and is a counterfactual.)

[japgolly]

I don't think it's splitting hairs to differentiate between "room for ambiguity" and "misleading," and think this is firmly the latter

I have to strongly disagree with you on that one. It is not universally misleading. Someone used to imperative code and using procedures rather than functions is going to see that and, with an implicit understanding, assume that the time of execution is immediate (one can argue that for such it should be named modStateImmediately or modStateNow); such a person would feel mislead. Someone else used to different conventions (esp typed FP practitioners) is going to look at that as a function that returns a Callback that describes the action of state modification; such a person would feel mislead (and near rabid) if the function side-affected.

Accrue from where? The only source I can think of is experience with/ the documentation of React, and I've twice tried to explain why that knowledge might not transfer the way you've described.

Apologies, I don't think I responded clearly enough to this point. This library doesn't change any of React's semantics. Syntax and structure change (like Callback), but the semantics are never changed. Maybe I'm too pedantic a person to expect that to clarify the situation for everyone? If so, I guess that's a doc change (?).

Probably the biggest reason I find modState's semantics confusing is that there are two separate layers of asynchrony whilst React has one. The extra indirection was very surprising; React's setState will, eventually, perform a state change, then execute a callback. modState will, eventually, tell react to, eventually, make a state change, and after it's done telling react to eventually make a state change, execute the >>-ed callback.

That's not quite right. Anything in a Callback is arbitrary code passed to React for it to execute zero or more times, in some kind of event callback (DOM or component). Whatever happens inside that callback happens immediately once React calls it. When it hits a setState or a modState it's the same as React; it will schedule the update then continue with the rest of the callback - this is no different in JS. As an additional feature, React allows an additional callback to be supplied along with the state modification, which it will call back once the state has been asynchronously applied. Again, this is how React itself works, and setState is the name under which it does it all. Callback in scalajs-react doesn't do anything, it just puts a name to the callback function you pass to React.

(Also, what should we retitle this issue to? My original title is horribly misrepresentative of the issue and what is being discussed and is a counterfactual.)

Do you feel that "Clarify semantics of set/modState" well represents what we're discussing?

[tgiddings]

I apologize for my inactivity on this issue. I became far more busy than I was expecting.

Someone used to imperative code and using procedures rather than functions is going to see that and, with an implicit understanding, assume that the time of execution is immediate (one can argue that for such it should be named modStateImmediately or modStateNow); such a person would feel mislead. Someone else used to different conventions (esp typed FP practitioners) is going to look at that as a function that returns a Callback that describes the action of state modification; such a person would feel mislead (and near rabid) if the function side-affected.

This is not what I meant. $.modState itself should not side-effect. I.e. I do not expect a function such as

def foo = {
  $.modState(_.copy(bar=true))
  if($.getState.map(_.bar)) println("baz")
}

to print "baz." I do not know of a way that would be possible with react. I would, however, expect code such as

val callback = $.modState(_.copy(bar=true)) >> $.getState.tap{state=>
  if(state.bar) println("baz")
}

to consistently print "baz." The expectation of a callback is that it is a function that will be executed at a time of React's choosing. The expectation of the >> is to "Sequence a callback after this" (from the scaladoc) I.e., once the the callback on the left is done, execute the callback on the right. My issue with modState is that it isn't "done" before the next callback executes.

Here is an analogy: suppose I made a javascript game where a button executes the following callback when clicked: playDeathAnimation>>showGameOverText. The expectation is that, when react feels like executing the callback, it will play the death animation. Then, when it is done playing the death animation, show the "Game over" text. Likewise, if the callback is $.modState(_.copy(_.dead=true))>>showAliveOrDeadSprite, the expectation is that, when React feels like executing the callback, it will modify the state. Then, when it is done modifying the state, show the alive or dead sprite. However, this is not what happens.

Someone else used to different conventions (esp typed FP practitioners) is going to look at that as a function that returns a Callback that describes the action of state modification

That is precisely what I expect. The action described by the Callback returned by $modState is not that of state modification. It is that of scheduling a state modification. $.modState is doubly asynchronous, but its name and type together indicate it being singly asynchronous.

That's not quite right. Anything in a Callback is arbitrary code passed to React for it to execute zero or more times, in some kind of event callback (DOM or component). Whatever happens inside that callback happens immediately once React calls it.

The code that creates the callback has no control over the instant that react calls the code in the callback. That is, by definition, asynchronous. The code inside the callback which calls React's native setState has no control over the instant that react sets the state, That is, by definition, asynchronous. Thus, setting a callback to $.modState is scheduling asynchronous code that schedules asynchronous code. Multiple layers of asynchrony is inherently unintuitive and, while often valid, needs to be advertised, hence my suggestion of something with "schedule" in the name (I'll retract my suggestion of "eventually" since that may become conflated with the inherent layer of asynchrony in a callback when it was intended to advertise an extra layer. I am wondering if that is what prompted your remark on side-effecting). If I change $.modState to $.scheduleStateMod in the above analogy, creating $.scheduleStateMod >> showAliveOrDeadSprite the meaning becomes "when React feels like executing the callback, it will schedule a state modification. Then, when it is done scheduling the state modification, show the alive or dead sprite," which is exactly what happens!

Elaborating more on your justification that "Whatever happens inside that callback happens immediately once React calls it" does not make it asynchronous: If I replace Callback with Future and "React" with "the executor," it becomes "Whatever happens inside that Future happens immediately once the executor calls it." That is exactly how normal asynchronous programming works. "the executor" is at total liberty to execute the Future now or a hundred years from now, perhaps in response to a trigger, in this thread or another thread. React has the same liberties with its callbacks (It could in theory use web workers to execute "on another thread").

This library doesn't change any of React's semantics. Syntax and structure change (like Callback), but the semantics are never changed.

Any form of (functional) lifting is a change in semantics, and that is exactly what Callback does (It even says so in the scaladoc). Concretely: Callback adds a layer of asynchrony around a function. React's native setState is an asynchronous function. Scalajs-react's setState and modState are asynchronous wrappers for setState, making them doubly asynchronous. You've already abandoned React's original semantics. Further, something with the same name as in the original language is expected to be the original thing (in whatever the new semantics are), not to lift it.

[japgolly]

My friend, you've rejected my explanations with no logical refutations of your own, and you've just doubled-down on your original opinions. In many places you've just strengthened your language and even used bold but your logic is still the same and often just an appeal to satisfy your expectations. That's not going to convince me of anything. From my perspective, I've already responded to all of your opinions, and I don't have anywhere near the time required to do so again, especially when it's just going to be the same as my previous reply. I appreciate the time you've taken to write up your views but I think it's time we agree to disagree.

[japgolly]

There's already mention of this in https://github.com/japgolly/scalajs-react/blob/master/doc/USAGE.md#gotchas

Considering that and that we already have CALLBACK.md and #419, I believe this can be closed.

[tgiddings]

I don't see an issue with rolling this into #419, but I read both of the documents you listed before making this issue and again after your closing and they do not address this. The closest is in CALLBACK.md in the section about the "executing instead of composing" mistake. The example is for composing a modState with printing to the console. The text that is printed to the console is "Scheduled state increment by 1" (emphasis added).

I'd also like to explain my silence:

• I was frustrated since I did not feel respected
• I was frustrated since I did not feel my statements were addressed
• I was incredible taken aback by your final comment
• I needed to put the project which used this library on hold (and thus I forgot about this issue)

Re-reading this issue after not looking at it for almost 6 months, I feel that that frustration was warranted. It reads as though, after a certain point, you skimmed through each of my comments and did not internalize any of what I said. I was desperately trying to rephrase my original remarks and to explain my issue from different angles in order to get a response which made sense with what I said, but that did not happen and I eventually rage-quit.

This isn't just a matter of disagreement; at my work I'm frequently told what amounts to "Your idea sucks and here's why", but it's still the single most respectful environment I've been in since their manner of disagreeing makes since with what I'd said to them. You yourself disagreed in a sensible way when I suggested specializing Callback. But that didn't happen when it came to me explaining why I found a specific aspect of the library to be incredibly unintuitive.

[japgolly]

I'm not your work and I don't get paid to argue with you. I do this entirely in my free time. To spend it in the Issues tab of github arguing in circles without any progress is a terrible waste of the time that I can spend on scalajs-react. If we were getting somewhere productive, then great, but I answered your concerns to the best of my ability and at least from my perspective, you keep reiterating the same points over and over again seemingly without hearing or comprehending my response. Enough time and patience on this now: if you'd like to see a change in the documentation, you submit a PR and I'll review it. Now if you didn't feel respected by our discourse, well firstly I'm sorry to hear that as it was my every intention to be respectful, but on the other hand it changes nothing in regards to this issue. You don't have a right to be agreed with (if that's what's leading you to feel disrespected) so maybe try different arguments, further consider my refutations and "internalise" them as you expect me to do with your arguments, and/or confront what seems to be your own sense of entitlement and readjust your expectations from a volunteer offering you years of free work on a free library (multiple really), and additional one-on-one free time volunteering to discuss this issue with you. We both argued in good faith but now the amount of time and effort has well exceeded the payoff.

Hmmm, it appears I'm similarly "taken aback" by your response. How harmonious.