CharlesMangwa / react-data-fetching

🎣 Declarative data fetching for React.
https://react-data-fetching.now.sh
MIT License
491 stars 27 forks source link

Cool project! refetch might want to be changed a bit #5

Closed jaredly closed 6 years ago

jaredly commented 6 years ago

currently you don't check if refetch is truthy -- it's only if refetch changes (code). I'd recommend renaming it to refetchKey or fetchKey -- it can tie into people's understanding of react's key prop. If the fetch key changes, then it will fetch again. Similar to how in react, if the key changes, then the component gets unmounted & remounted.

CharlesMangwa commented 6 years ago

Hi @jaredly! Thanks for taking the time to make this feedback. You get a point here: the fact that refetch doesn't necessarily have to be a boolean isn't really obvious given its name. refetchKey seems to be a good alternative (and making the relation with React's doc could indeed help for a better understanding)! I'll change this in the next release.

ad1992 commented 6 years ago

@CharlesMangwa cool project :) I would like to contribute in the project. Are you working on this or can I take it up ?

CharlesMangwa commented 6 years ago

If you can make the changes and send a PR @ad1992, that would be awesome! 🙌

ad1992 commented 6 years ago

@CharlesMangwa sure. I ran npm install,npm run build. Can you please guide me on how to get started in Dev mode

CharlesMangwa commented 6 years ago

Hi @ad1992! Sorry for the lack of guidance, I should probably update the README. Long story short: you just have to clone the repo, run npm install (yarn install would be better if you want to match the project's yarn.lock), make the changes, and submit your PR. There's no need to run npm build given that this script is just used to general the final packages that are going to be published on npm, so you don't need to run this command :). Hope things are more clear now!

ad1992 commented 6 years ago

@CharlesMangwa thanks for the reply :) but I think you didn't get my question. After making any change if i want to test it then should I make the change in test and check or is there any other way any change in the code base can be tested ? is there any file where it can be tested like sometimes projects have example folder to try out the changes. So wanted to know if I want to test after making any change to the code then what is the best way to do it.

CharlesMangwa commented 6 years ago

@ad1992 Oh! Little misunderstanding here indeed. Right now I don't have a proper, "official" way to test it, but what I used to do is to test it inside personal React.js & React Native projects. I should probably think about a more efficient way to do this for future contributors. However, I don't think that your changes (if we're still speaking about refetch being renamed in refetchKey only) could lead to a dysfunction. But you can also run yarn test before committing, that could help!

ad1992 commented 6 years ago

@CharlesMangwa yes you got it right. May be an example folder can be added which has a demo react app where the functionalities can be tested. I agree not for this change but for any core change which needs testing, it would be great to have a demo app where it can be easily tested.