Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

suggestion on how to fix the casting problems #10

Closed
wants to merge 1 commit into from
Closed

suggestion on how to fix the casting problems #10

wants to merge 1 commit into from

Conversation

jarlah
Copy link
Contributor

@jarlah jarlah commented Mar 16, 2019

No description provided.

@dai-shi
Copy link
Owner

dai-shi commented Mar 17, 2019

Hi, I appreciate for your consideration.
Not being sure if I understand the comments over time in #8, how much does this solve your issue?

My intuition for this PR is a bit hesitation. If the issue is about TypeScript type inference which might be improved in the future, I don't want to change index.js. Type casting should be better.

@jarlah
Copy link
Contributor Author

jarlah commented Mar 17, 2019

Ill test the reducer cast :) maybe i didnt like casting the function. Casting parameters is more logical. And more readable. I think.

@jarlah
Copy link
Contributor Author

jarlah commented Mar 17, 2019

ill use the function cast :)

@jarlah jarlah closed this Mar 17, 2019
@jarlah jarlah deleted the suggestionOnHowToFixCasting branch March 17, 2019 07:56
@dai-shi
Copy link
Owner

dai-shi commented Mar 17, 2019

Glad to hear that.

maybe i didnt like casting the function. Casting parameters is more logical.

Just a note: I would like to avoid casting at all if possible. Casting is just an escape hatch.
I tried hard to make the Enhancer type compatible with Redux, but it wasn't successful.

@jarlah
Copy link
Contributor Author

jarlah commented Mar 17, 2019

Casting to supported type is ok i think. Casting to any is no no. If i try to cast it to String i get error. So there is safety here. :) but anyway we can continue discussion in the open issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants