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

Docs - add 'Migrating to RTK Query' page #1060

Conversation

Shrugsy
Copy link
Collaborator

@Shrugsy Shrugsy commented May 16, 2021

Related to #964

  • Adds a 'Migrating from thunks' 'Migrating to RTK Query' page

Note that realistically it is more like 'How to delete your thunk code and re-write it shorter with RTK Query'

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 16, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 09a6531:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration
rsk-github-issues-example Configuration

@netlify
Copy link

netlify bot commented May 16, 2021

Deploy Preview for redux-starter-kit-docs ready!

Built with commit 09a6531

https://deploy-preview-1060--redux-starter-kit-docs.netlify.app

@markerikson
Copy link
Collaborator

markerikson commented May 16, 2021

A couple suggestions:

First, let's retitle it "Migrating to RTK Query". I don't think we need to specifically show examples of switching from sagas and observables also, but let's keep the title more general.

Next, I'd suggest starting with a brief recap like:

The most common use case for side effects in Redux apps is fetching data. Redux apps typically use a tool like thunks, sagas, or observables to make an AJAX request, and dispatch actions based on the results of the request. Reducers then listen for those actions to manage loading state and cache the fetched data.

RTK Query is purpose-built to solve the use case of data fetching. While it can't replace all of the situations where you'd use thunks or other side effects approaches, using RTK Query should eliminate the need for most of that hand-written side effects logic.

@Shrugsy
Copy link
Collaborator Author

Shrugsy commented May 17, 2021

Thanks @markerikson, I've made those changes

@Shrugsy Shrugsy changed the title Docs - add migrating from thunks page Docs - add 'Migrating to RTK Query' page May 19, 2021
@Shrugsy Shrugsy force-pushed the docs/add-migrating-from-thunks-page branch from 4ca0f96 to 6429f1e Compare May 20, 2021 07:48
@markerikson
Copy link
Collaborator

Content-wise, this doesn't really feel like a "migrating" page at the moment.

The examples section is more like "here's two separate examples, one without RTKQ and one with RTKQ". I'd like to see more of a "here's our starting point with just RTK, and here's the individual steps I'd take to convert the RTK code to RTKQ", similar to what I have in https://redux.js.org/tutorials/fundamentals/part-8-modern-redux for going from handwritten Redux logic to RTK.

@Shrugsy
Copy link
Collaborator Author

Shrugsy commented May 21, 2021

@markerikson I've re-written it with a more 'step-based' approach. Let me know your thoughts

@Shrugsy
Copy link
Collaborator Author

Shrugsy commented May 21, 2021

Actually I've just realised this snippet is wrong (regarding the 'status' check)

    const response = await fetch(
      `https://pokeapi.co/api/v2/pokemon/${name}`
    ).then((res) => res.json())
    if (response.status < 200 || response.status >= 300) {
      return rejectWithValue(response)
    }

I'll fix that up later on

@markerikson
Copy link
Collaborator

Looks fairly good. We could probably make more improvements, but at this point I just want to get some reasonable content in place for any remaining topics so we can launch.

@markerikson markerikson merged commit 62bc12d into reduxjs:feature/v1.6-integration May 21, 2021
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

Successfully merging this pull request may close these issues.

2 participants