search

cassiozen / useStateMachine

The <1 kb state machine hook for React
MIT License
2.38k stars 47 forks source link

Porting txstate #56

Closed devanshj closed 3 years ago

devanshj commented 3 years ago

Supercedes #36

I was trying to write some types for implementation for you so I even rewrote the whole thing because I felt like and I was bored ahaha. Take no pressure in merging this but if you like it do it and you can also use the types from here.

There's some really good stuff in here that is branded types and assertNever for exhaustive check and strongly typed record which I don't have time to explain because I'm going to sleep :P but the gist is as all primitive are branded you can't make mistakes like these...

definition.states[event.type] // nope
definition.states["foo"] // nope
R.get(definition.states, machineState.value) // the only way to access the node
R.get(definition.states, transition.target) // also works till the time the type is the same

let x = machineState.value;
x = event.type // nope both are string in runtime but aren't of the same type so can't be assigned.

Haven't checked if the tests are passing or not btw didn't have time was in a rush. Coz really I just rewrote this for fun don't mind me xD (this branch is based on txstate-port btw, if you want to quickly check the diff you can do it here). And if it interests you feel free to ask questions.

And also if you like those Impl types let me know if you want me to add them to txstate-port branch itself.

To-do:

cassiozen commented 3 years ago

Hey @devanshj - I just wanted to say that this is awesome! I'm on crunch time at work so I didn't have time to review It yet, but I'm super excited to merge!

devanshj commented 3 years ago

Thanks I'm glad you liked it! I didn't even expect you to want to merge it haha. And no worries at all take your time, I understand.

cassiozen commented 3 years ago

Ok, I started reading the code. I will probably more questions as I go ;)

  1. Is you extras/R kind of a tiny TS Rambda of sorts?
  2. This version doesn't yet support nested states, correct?

Some other observations & things to do:

  1. TSDX (the setup tool used in use-statemachine) doesn't support TypeScript 4 yet, but there's a workaround using yarn resolutions - I updated the project to use it, so make sure to use yarn from now on.
  2. This version doesn't have a specific log for when a transition is blocked by a guard and doesn't log successful transitions. I can work on that.
  3. The minimized bundle is now bigger than half a kb (even using brotli instead of gzip), so maybe we should rename the catch-phrase from "The ½ kb state machine hook for React" to "The <1 kb state machine hook for React" 😂
  4. Loved assertNever.
  5. Need to update examples and failing tests. I can also work on those
  6. Need to update docs and work on a release article. Can we split the work on this? Do you have the availability?

Overall I love how this is going. You put a lot of effort into this, so I'd like to invite you to be a maintainer of this project with me. Is this something that interests you?

devanshj commented 3 years ago

Is you extras/R kind of a tiny TS Rambda of sorts?

Not really, R stands Record. It only has functions you'd use on an object used as a map. So instead of a[k] you'd write R.get(a, k) and instead of Object.keys(a) you'd write R.keys(a). The reason is that typescript doesn't allow indexing via branded string, so now { [_ in string & { __someTag: true }]: "foo" } is actually { __K: string & { __someTag: true }, __V: "foo" } instead. Let me know if you want me to elaborate

This version doesn't yet support nested states, correct?

That's right

TSDX (the setup tool used in use-statemachine) doesn't support TypeScript 4 yet, but there's a workaround using yarn resolutions - I updated the project to use it, so make sure to use yarn from now on.

Sure

This version doesn't have a specific log for when a transition is blocked by a guard and doesn't log successful transitions. I can work on that.

Ah must have missed, thanks.

The minimized bundle is now bigger than half a kb (even using brotli instead of gzip), so maybe we should rename the catch-phrase from "The ½ kb state machine hook for React" to "The <1 kb state machine hook for React" 😂

Haha still has a lot of useless stuff like assertNever can be an empty function we need not throw the error it's only because we get compile-time exhaustive checks.

Loved assertNever.

Haha thanks, idr if it was original or inspired xD

Need to update examples and failing tests. I can also work on those

Yep that'd be good.

Need to update docs and work on a release article. Can we split the work on this? Do you have the availability?

Sure thing, I'd prefer to work on a release article. I was thinking of hosting it somewhere (as opposed to md) so we can have twoslash intergration (as opposed to screenshots) but then I think that'd be too much of work :P wdyt?

You put a lot of effort into this, so I'd like to invite you to be a maintainer of this project with me. Is this something that interests you?

I understand where you're coming from but it's not something that I'd opt into majorly because this is something I do for fun quite literally and maintaining a library is no fun xD Look I understand that contribution comes with an implicit responsibility which I'll gladly fulfil (eg helping you out if users report some ts issues etc) but "maintainer" is a overhead responsibility for me especially given the area this library focuses is not really of my interest, I've never written a state machine nor have I even used xstate, it just happened to be a typing challenge that I took. So thanks for the offer but I don't think I'll be able to take it sorry!

devanshj commented 3 years ago

Other things pending here -

  1. rebase to txstate-port (there are some changes I did)
  2. See if we can have types ContextImpl instead of Context.Impl because the intellisense quickinfo tooltip shows the type of everything as Impl which is hella annoying and useless
cassiozen commented 3 years ago

So thanks for the offer but I don't think I'll be able to take it sorry!

Don't worry, I understand.

Sure thing, I'd prefer to work on a release article. I was thinking of hosting it somewhere (as opposed to md) so we can have twoslash intergration (as opposed to screenshots) but then I think that'd be too much of work :P wdyt?

I think you should absolutely do it. Is there any public platform that supports twoslash (like dev.to)?

devanshj commented 3 years ago

Don't worry, I understand.

Thanks for understanding!

Is there any public platform that supports twoslash (like dev.to)?

I doubt. We can use gh-pages I guess. I could use remark-shiki-twoslash or something like that to output html.

cassiozen commented 3 years ago

Question: Prettier is screaming at me when I open the files. I see that you like to be precise about the formatting - can you live with some configuration of Prettier? We can disable it too, but I would prefer to keep it.

devanshj commented 3 years ago

I see that you like to be precise about the formatting

Good observation :P I was about to mention about prettier, I tell you what -- I absolutely hate it. I could write an essay on how it's just wrong. But hey it's your project if you prefer to keep it do that. Though I doubt it'd matter as it's not like a big org where people would have diff opinion about formatting and would bike-shed all the time (that's the only place I can give prettier a benefit of doubt). Though again I'd have you take the call.

Oh wait yeah I would still want prettier to ignore types.ts at least or it'd make things absolutely unworkable.

devanshj commented 3 years ago

Did you rebase to the latest txstate-port?

cassiozen commented 3 years ago

I tell you what -- I absolutely hate it. I could write an essay on how it's just wrong

🤣😆

I'd have you take the call. (...) I would still want prettier to ignore types.ts

Deal - I'll keep ignoring types.ts and I left the prettier requirements less strict, but I'll keep on index and extra.

cassiozen commented 3 years ago

I'm having a problem with guards on this branch: Whenever I add a guard to a transition, its expected type is undefined:

guard_undefined
cassiozen commented 3 years ago

Did you rebase to the latest txstate-port?

I did not

cassiozen commented 3 years ago

I edited the first comment to add a bunch of to-do tasks - hope you don't mind.

devanshj commented 3 years ago

I'm having a problem with guards on this branch: Whenever I add a guard to a transition, its expected type is undefined:

Yeah I noticed and fixed it on txstate-port (I'll rebase to that). Either I missed it or maybe regression from 4.3.2 to 4.3.5 not sure

devanshj commented 3 years ago

I edited the first comment to add a bunch of to-do tasks - hope you don't mind.

No that's great we'd have clarity!

devanshj commented 3 years ago

I wanted to keep txstate-port to as source of truth for types.ts so I cherry-picked the type changes from rewrite (the Impl types etc) to txstate-port then rebased rewrite to txstate-port and finally force pushed, hopefully I didn't mess up anything :P

Edit: I think I'll work on this branch for types too

cassiozen commented 3 years ago

Hey @devanshj - Do you think this is in a state we could release another beta before the official 1.0 release? It could be nice to have some additional user testing.

cassiozen commented 3 years ago

Found a bug while converting the timer example. Filled separately for better organization: https://github.com/cassiozen/useStateMachine/issues/62

devanshj commented 3 years ago

Do you think this is in a state we could release another beta before the official 1.0 release? It could be nice to have some additional user testing.

Sure thing but not yet, I'll mark this a draft for clarity and will open once it's ready or at least ready enough for a beta. Right now enforcing t and disallowing $$initial as an event type are remaining then we'd be ready for another beta.

devanshj commented 3 years ago

@cassiozen I think this is good for another beta.

cassiozen commented 3 years ago

I think this is good for another beta

Awesome. I created a new branch 1.0.0-beta and rebased this PR to merge and bake a new release from the original repo.

We can continue working on this same PR on your fork, or open a new PR, whatever you prefer.

cassiozen commented 3 years ago

@all-contributors please add @devanshj for bug, code, ideas and test

allcontributors[bot] commented 3 years ago

@cassiozen

I've put up a pull request to add @devanshj! :tada:

devanshj commented 3 years ago

The only thing pending other than documentation is lite-types, I think I'll open a new PR for that and base it on the beta branch. You can add commits changing documentation on the beta branch directly if you want.