Api changes for sendT

[devanshj]

machine = { ...machineInstant, send }
Machine.State -> Machine
machineState -> machineInstant
machineState.value -> machineInstant.state
Machine.StateValue -> Machine.State

[cassiozen]

Interesting, not what I was expecting. I thought we were going for {machine, state} = useStateMachine(...) instead of {state, context, nextEvents, send} = useStateMachine(...).

[devanshj]

Ha I thought it was apparent in the article haha. To explain why it's like that we need the discriminating property one level deep make the narrowing happen, hence machine.state === "editing" && machine.send("DISCARD") would work.

With your version, state === "editing" && machine.send("DISCARD") won't narrow machine something.state === "editing" && something.machine.send("DISCARD") would but the whole thing looks too complex.

Moreover with shouldSendT we might narrow the machine via this type predicate as a discriminating union would no longer work so to make machine.state === "editing" && machine.shouldSendT("POST") && machine.sendT("POST") work shouldSendT might be something like...

shouldSendT(this: X, event: Y): this is Z

And hence the users should probably not use destructuring as it might make the type predicate not work.

Moreover I'm not sure if machine.state === "editing" will suffice we might have to opt for machine.hasState("editing") with the same this predicate.

I haven't really tried any of this out though haha probably I should.

[cassiozen]

Moreover I'm not sure if machine.state === "editing" will suffice we might have to opt for machine.hasState("editing") with the same this predicate.
I haven't really tried any of this out though haha probably I should.

Can you try it before we merge?

Another question: Can I update the existing examples to use rest in the destructuring? Like:

const {send, ...machine} = useStateMachine({...})

[devanshj]

Can you try it before we merge?

Yep will do.

Can I update the existing examples to use rest in the destructuring?

I'd say prefer not to, because in typescript's eyes send (and shouldSend) is somewhat on the prototype (hence should not be destructured) as it would probably leverage this. I can't comment concretely before I have tried things. I'll give an update after I've tried.

[devanshj]

There... Will have to introduce hasState to have these error messages (as opposed to simply not accepting "Y")

And destructuring obviously won't work nor typescript would allow it at places I've defined this

Though we can't have the error messages that I wanted to have like Error: Y can be only sent when in b or something like that because while the time of showing error the argument is not inferred as "Y" so to have error like those we'd tell users to write sendT.debug("Y") which would return the error as a string and then sendT would simply not suggest "Y" nor it would show errors.

Anyway, the point was to see if we can leverage this guard and we can, then in that case some of these become methods on prototypes for TypeScript (we don't actually need to have them on the prototype in runtime) so we can't use destructuring as it won't work as shown above.

EDIT: Wait given that folks will have to use hasState to leverage sendT should we add it already? Or add it with sendT?

[cassiozen]

It feels like hasState has a lot of value even without sendT, so I think we should move forward and implement it.

[devanshj]

We should implement hasState but it has literally no value without sendT because m.hasState(x) is same as writing m.state === x. The only time m.hasState(x) and m.state === x would differ is with usage of sendT because hasState narrows m but m.state === x doesn't (sufficiently enough to make sendT work).

The reason I was suggesting we should already have hasState is so that users start writing m.hasState(x) instead of m.state === x so that when we have sendT they can leverage it right away instead of first migrating m.state === x to m.hasState(x).

So I guess first we should merge this and then I'll open a separate PR for hasState? Actually before hasState I'd open a PR for createDefinition so as to close #78

Edit: Oh I realized this has conflicts haha will resolve

[cassiozen]

Ohh, got it. Yeah, I think it might make sense to release it with sendT then.

[devanshj]

Will resume working on the documentation once this (that includes #76) is released so as to not redo the work as the api has changed

[cassiozen]

Just released it, @devanshj

[devanshj]

Great! Could you merge and release this PR too? (As this PR has the finalized api to be documented)

[cassiozen]

Hey @devanshj,

Since I've been stuck as work for a while and the library have been steadily increasing, I reverted back and released Beta 3 as 1.0.0.

I'm reopening this, we can further discuss releasing it as 2.0.0

[devanshj]

Sure no problems, indeed we can release it as v2.

I've been busy too haha, looking forward to continue working on this library!