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

Add the ability to .case on a string #24

Open
MJLHThomassen-Eurocom opened this issue Mar 5, 2018 · 2 comments
Open

Add the ability to .case on a string #24

MJLHThomassen-Eurocom opened this issue Mar 5, 2018 · 2 comments

Comments

@MJLHThomassen-Eurocom
Copy link

I'm currently working with react-router-redux which defines a LOCATION_CHANGE, however since the library is not using typescript-fsa-reducers, i can't .case() on that action in a reducer.

I want to clear an error message string on every LOCATION_CHANGE action. As a workaround, I have created a "new" actionCreator(LOCATION_CHANGE) in my list of actions for the control this concerns, however, perhaps there is a way to also allow external actions in a .case statement? This would obviously mean loosing type safety. However, react-router-redux does define the propper LocationChangeAction action interface, so perhaps this can be solved with generics?

@dphilipson
Copy link
Owner

Ah yes, this makes sense as a use case. As you said it does lose type safety, but that can't really be helped. I don't know if generics are particularly helpful because

.case<FooAction>("FOO", (state, action) => { /*...*/ })

isn't any safer than

.case("FOO", (state, action: FooAction) => { /*...*/ })

but it's probably fine to be untyped when dealing with untyped external libraries. In my own code I would probably create the actionCreator(LOCATION_CHANGE) like you described, but I can see the value in making it possible to skip this step as well.

I'll try to make a change for this some time this week.

@MJLHThomassen-Eurocom
Copy link
Author

Thanks!

You are right about both examples you give being equally typesafe, but it would be desirable to be able to do it (e.g. not restrict action to AnyAction but to T extends AnyAction) obviously.

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

No branches or pull requests

2 participants