search

sheaivey / react-axios

Axios Components for React with child function callback
MIT License
180 stars 20 forks source link

improve `withAxios` hoc experience #27

Closed achepukov closed 5 years ago

achepukov commented 5 years ago

Hello, you've a great library, really appreciate it! Hovewer, withAxios hoc have poor experience, my PR is about to improve it. New features are described in Readme.md. I think it's major version update as hoc signature is changed from withAxios(Component) to
withAxios(opts)(Component) I also updated some babel stuff, as it looks obsolete. Feel free to comment/question if any. Thanks!

sheaivey commented 5 years ago

@achepukov Great idea to make react-axios also work as an HOC. Please allow me some time to review your implementation. Thanks!

achepukov commented 5 years ago

@sheaivey I've just found that my implementation doesn't use logic about canceling a request that default components have... I'd note that in Readme.md, and create separate project for that (as it seems to be a big refactoring). And as anyway, my PR improves the hoc experience, then I'd suggest to merge it first, and next wait untill somebody adds PR about improve the HOC so it uses cancel function as well.

sheaivey commented 5 years ago

Thanks for the follow up @achepukov. After looking at your implementation I thought it could be done while maintaining the existing Request composition. I created a branch that could be deployed without a major version bump.

Have a look here for my version of withAxios implementation. https://github.com/sheaivey/react-axios/blob/withAxios/src/components/withAxios.js

I think this takes into account what your PR is trying to accomplish but with more code reuse since withAxios(options)(Component) is just a thin wrapper to the Request component. By doing this we don't need to reimplement debounce, canceling and a lot of other helpful features. The Request child call back function spreads error, response, isLoading, onReload, axios, options onto the component being wrapped for later consumption. The initial withAxios options can be overloaded at any time by passing options={{}}.

Let me know if there is anything missing that you think would be beneficial to the HoC.

sheaivey commented 5 years ago

I created PR #28 which you are welcome to make PR against.

Thanks!

achepukov commented 5 years ago

@sheaivey, great, it's definitely correct direction! Gimme some time, as I need to think on how everything will work that way.