processing / p5.js-web-editor

The p5.js Editor is a website for creating p5.js sketches, with a focus on making coding accessible and inclusive for artists, designers, educators, beginners, and anyone else! You can create, share, or remix p5.js sketches without needing to download or configure anything.
https://editor.p5js.org
GNU Lesser General Public License v2.1
1.3k stars 1.26k forks source link

refactor collections reducers and actions using redux toolkit #3125

Open PiyushChandra17 opened 1 month ago

PiyushChandra17 commented 1 month ago

Associated issue: #2042

Changes:

I have verified that this pull request:

raclim commented 2 weeks ago

Thanks for your work on this! I'm testing this out by running a branch and I think all the functionality generally works except for the one for deleting a collection. Would you be able to update this PR so its functionality mirrors the one that's currently up?

PiyushChandra17 commented 2 weeks ago

Thanks for your work on this! I'm testing this out by running a branch and I think all the functionality generally works except for the one for deleting a collection. Would you be able to update this PR so its functionality mirrors the one that's currently up?

@raclim Sure!

PiyushChandra17 commented 2 weeks ago

@raclim Updated this PR to delete a collection, can you please verify once if it's working? Thanks!

PiyushChandra17 commented 2 weeks ago

This is fine I guess. It's no worse than what we have now but I do think that there are other "tools" in the Redux Toolkit that can be more helpful, such as createAsyncThunk and RTK Query. I don't know how much refactoring @raclim is comfortable with, but it seems like we're side-stepping the actual pain points here. The reducer was and still is very simple, it's the thunk actions that are messy.

We can use createAsyncThunk where we define the main asynchronous action (the API call) and it automatically dispatches "loading" "success" and "error" actions for it. Our existing loading reducer only looks for actions with the exact type of startLoader and stopLoader, but we could update it to look for any of the async thunk action types using matchers.

We still would not know what is loading. This has caused issues before such as #2250 and I've had to put in work-arounds #2996 (comment). That is where RTK Query can help. We would define an "endpoint" for each of these actions. It automatically manages the Redux state, with a separate status for every API call. In the components we would write something like const { data, isError, isFetching } = useGetCollectionsQuery(username).

@lindapaiste Exactly! we can create one apiSlice file and use it in apiSlice.injectEndpoints for every actions. Yes we need to follow this convention useGetCollectionsQuery and if it's mutation then instead of query, it would be mutation and we use these in the components. As far as isLoading, isError, isFetching is concerned we can destructure them and use it right out of the box on a fly.