search

the-dr-lazy / deox

Functional Type-safe Flux Standard Utilities
https://deox.js.org
MIT License
206 stars 12 forks source link

fix: use symbol as a key for the "others" handler #159

Closed Jazzmanpw closed 3 years ago

Jazzmanpw commented 3 years ago

I believe the work is mostly finished. I only have one typing issue, but I'm not sure if it's a problem. When I run dts tests, one for createReducer falis (it worked previously, but some other types were not correct). In this line it determines the action type as any.

I don't know if there's any use of an exact action type for a reducer and assume that such behavior shouldn't be dangerous. As well as it affects only the new handle.others feature, so it's back compatible.

My question is: is it OK to have the any type there, or should I try and find some way for a better structure for type inferring?

Jazzmanpw commented 3 years ago

And maybe I need to create a new fork just for the last commits, don't I?

the-dr-lazy commented 3 years ago

And maybe I need to create a new fork just for the last commits, don't I?

Maybe pull the latest master or next branch.

Jazzmanpw commented 3 years ago

I believe I screwed up this pull request a little bit. I found how to pull the changes from the original repo, but they're now all in this pull request. I've added a couple of commits before the pull and one that you've requested. Is it convenient to merge these changes? The Files changes tab looks a little bit messy

Jazzmanpw commented 3 years ago

The original question about any is still actual.

the-dr-lazy commented 3 years ago

I've added a couple of commits before the pull and one that you've requested

You should pull the upstream with rebase.

the-dr-lazy commented 3 years ago

is it OK to have the any type there, or should I try and find some way for a better structure for type inferring?

It's important to have correct type for created reducer. I know what is the problem. The data type for HandlerMap is wrong.

the-dr-lazy commented 3 years ago

You should pull the upstream with rebase.

I fixed the branching problem. Sorry, but I should force push to fix it.

the-dr-lazy commented 3 years ago

Please let me know your opinion about the changes I have made.

the-dr-lazy commented 3 years ago

I think this PR is ready to merge and release. Hope you think so @Jazzmanpw.

Jazzmanpw commented 3 years ago

I'm sorry for being away for so long. Thank you for the changes. I still don't feel comfortable with TS. And I'm not sure if I could do it better. As well as I don't see any remaining bugs.

the-dr-lazy commented 3 years ago

Thank you for your contribution @Jazzmanpw. If you are interested in type-level programming in statically typed languages such as TS which inspired their type system from ML languages especially Haskell, I suggest you learning Haskell. It's the best teacher I ever had.

Jazzmanpw commented 3 years ago

Thank you for the advice. And for the opportunity to become a part of the open-source community. =)