search

the-dr-lazy / deox

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

feat: default handler in createReducer #153

Closed Jazzmanpw closed 4 years ago

Jazzmanpw commented 4 years ago

related to #152

codecov[bot] commented 4 years ago

Codecov Report

Merging #153 into master will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #153   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines           56        58    +2     
  Branches         9        10    +1     
=========================================
+ Hits            56        58    +2
Impacted Files Coverage Δ
src/create-handler-map.ts 100.00% <100.00%> (ø)
src/create-reducer.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9925130...7258ef7. Read the comment docs.

Jazzmanpw commented 4 years ago

@the-dr-lazy, it seems to fail on running yarn run docs:build, but locally it works fine (it doesn't crush at least). I found an article in Netlify blog about such crushes, but I'm not sure if I should tweak your CI. May I ask you to check what's wrong? This is the link to the post I found. This and some other similar posts say that errors are caused by yarn.lock/package-lock.json and some wrong stuff with node_modules committed to the repo, but I didn't change anything about the project dependencies.

the-dr-lazy commented 4 years ago

What is your opinion about renaming handleAction.default into handleAction.otherwise? I think the default word can become ambiguous with default state in Redux and users think it will call when the state is equal to the default state they have sent in reducer. Looks like otherwise makes much more sense here.

the-dr-lazy commented 4 years ago

@the-dr-lazy, it seems to fail on running yarn run docs:build, but locally it works fine (it doesn't crush at least).

The issue has been fixed in the next branch by 8df47a040941ab3dfa90535d10b6562b5a6bb123.

Jazzmanpw commented 4 years ago

What is your opinion about renaming handleAction.default into handleAction.otherwise? I think the default word can become ambiguous with default state in Redux and users think it will call when the state is equal to the default state they have sent in reducer. Looks like otherwise makes much more sense here.

If we may discuss it, I think, otherwise might not be the best option. handleAction(action), handleAction(action), handleAction.otherwise This is what we'll receive in the end. Maybe, one of these versions will be more straight-readable: handleAction(action), handleAction(action), handleAction.others handleAction(action), handleAction(action), handleAction.rest

As well as, my IDE suggests handle as an argument for createReducer callback. Then handle.otherwise is clear, but a little bit off the sentence. Although I'm not an English native, so my comment on readability might not be relevant for people closer to the English language.

the-dr-lazy commented 4 years ago

Thank you for your comment. I'm with handleAction.others. Also, I think it's valuable to mention in functional programming there is a technique called wildcard for pattern matching. and Deox is a representation of pattern matching for JavaScript. This PR is trying to mention this technique.

Jazzmanpw commented 4 years ago

I've not heard about wildcard in FP, but I can see that the concept is pretty straightforward and familiar (we've all used switch ... default). Should it somehow affect the code, or is it just an idea to mention?

the-dr-lazy commented 4 years ago

Thank you for your contribution @Jazzmanpw. Nice job.

the-dr-lazy commented 4 years ago

:tada: This PR is included in version 3.3.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

Jazzmanpw commented 4 years ago

Wow, wait a moment. I appreciate that my job was useful, but I haven't finished it yet. I met some troubles with using a symbol as a key in the final handler map. The typing troubles that is. I believe I should finish it first, don't I?

the-dr-lazy commented 4 years ago

As this PR was not a draft I thought your work is done. Sorry for that. Please continue your work on another PR.

Jazzmanpw commented 4 years ago

I'm sorry about the PR status. I don't know much about it yet. I continue my work.