-
Notifications
You must be signed in to change notification settings - Fork 14
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 support for injecting a custom argument into epics #26
base: dev
Are you sure you want to change the base?
Conversation
Raised in issue joshburgess#19 but dismissed in favour of a more elegant HOF approach, after four months with no project activity, I've gone for the zero-impact pragmatic solution
(there's some travis config issue - the tests should pass!) |
Hi, @alex2 . I will check this out very soon. Unfortunately, I've been pretty busy lately. I've been meaning to get back to this and update the project to use most/core, include Flow types, explore the HOFs & this dependency injection feature. I just haven't had time recently. Thanks for submitting the PR. I'd love more help, in general! I'm currently interviewing for new jobs and very busy... I have some take home work to complete this weekend, but I may have a bit of time free if I finish it soon enough. Otherwise, it will be soon after. Sorry for the wait. |
No worries, really good of you to let me know. Good luck with your interviewing! |
@alex2 After reviewing, yes, I am willing to accept this. Here are the remaining tasks that need to be accomplished:
Let me know if you're okay with tackling these things. I'm okay with doing any or all of them if you'd prefer, but I figured I'd leave it to you if you'd like to contribute. Thanks again for taking the initiative to make a PR for this! I had been neglecting the project for a bit too long. |
@alex2 You still interested in helping finish this up (docs + type definitions) or would you like me to merge as is and help finish it? |
Hey, I've also been interviewing for a new job(!) but managed to be much less communicative than you while doing it, so my apologies for the lack of updates. I'll have some free time this weekend to make your suggested changes. |
@alex2 You should do a pull. The type definitions file recently changed somewhat due to someone's pull request to make it compatible with Redux 4.0. The changes are minor, but there are some conflicts right now. FWIW, I'll wait until later with this, but I've been thinking about changing the API soon to just use object destructuring to pass all params at the same time as one object arg. Like It makes a point-free style harder/less readable in some cases, but removes the positional aspect of the arguments and lets you just treat it as a grab-bag, letting you grab the things you need and ignore the rest. |
Hi, after using the project for a bit I'm really missing the dependency injection from thunk and redux-observable for my testing.
Implementing this was raised in issue #19 but dismissed at the time in favour of a more elegant HOF approach and imminent API changes, but after four months with no project activity I've gone for the simplest, most pragmatic implementation that I believe is backwards-compatible.
If you're willing to accept this (until the lovely proposed HOF-support rewrite arrives) let me know and I'll update the documentation too.