Skip to content

Elaborate on the documentation of preloadedState#2145

Open
dggsax wants to merge 2 commits intoreduxjs:masterfrom
dggsax:gonzo/preloadedStateDocs
Open

Elaborate on the documentation of preloadedState#2145
dggsax wants to merge 2 commits intoreduxjs:masterfrom
dggsax:gonzo/preloadedStateDocs

Conversation

@dggsax
Copy link
Copy Markdown
Contributor

@dggsax dggsax commented Mar 18, 2022

In the Reactiflux discord, I asked the following question:

"Is there a reason why the PreloadedState typing is not made such that it's recursively partial? For my testing, I would like to be able to set a value for part of a piece of state, such as controlling showNavigationPanel but allowing the initialState for the slice go ahead and define the theme:"

const initialState = {
  appState: {
    theme: "Dark",
    showNavigationPanel: true
  }
}

// preloaded state for test, I only care about controlling showNavigationPanel, I want theme to just be handled by the initialState for the slice
const preloadedState = {
  appState: {
    showNavigationPanel: false
  }
}

To which @markerikson replied "Your slice reducers will be given the pieces of state you provide at startup. Those can't be partial themselves.". Which is a pretty obvious answer if you think about it.

With this PR, I hope to provide some elaboration on preloadedState in the documentation so that others who come to this conceptual question can achieve the same understanding. This is my first attempt at contributing to the RTK documentation, so please let me know if you have any feedback on how I can make the explanations/examples more concise.

}

// Not correct, we don't provide a value for `theme`. You would get the following typescript error if you tried to assign preloadedState the `PreloadedState<RootState>` type: Property 'theme' is missing in type '{ showNavigationPanel: false; }' but required in type '{ theme: "Dark" | "Light"; showNavigationPanel: boolean; }'.
const incorrectPreloadedState = {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@phryneas is there a way that I can give this the type PreloadedState<RootState> and have the type error be displayed in the documentation without breaking the compilation of the MDX?

@netlify
Copy link
Copy Markdown

netlify Bot commented Mar 18, 2022

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

🔨 Explore the source changes: 5f22b97

🔍 Inspect the deploy log: https://app.netlify.com/sites/redux-starter-kit-docs/deploys/6235122abb1a3c000851cabe

😎 Browse the preview: https://deploy-preview-2145--redux-starter-kit-docs.netlify.app

@codesandbox-ci
Copy link
Copy Markdown

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 36342d9:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration
rsk-github-issues-example Configuration
@examples-query-react/basic Configuration
@examples-query-react/advanced Configuration
@examples-action-listener/counter Configuration

@netlify
Copy link
Copy Markdown

netlify Bot commented Mar 18, 2022

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

🔨 Explore the source changes: 36342d9

🔍 Inspect the deploy log: https://app.netlify.com/sites/redux-starter-kit-docs/deploys/623512d412d4cf0008fba3c0

😎 Browse the preview: https://deploy-preview-2145--redux-starter-kit-docs.netlify.app

@dggsax dggsax marked this pull request as ready for review April 25, 2022 19:14
@dggsax
Copy link
Copy Markdown
Contributor Author

dggsax commented Apr 25, 2022

Publishing this PR to start getting some feedback on what I've written here.

Here's the direct link to preview the changes that I've made in this PR: https://deploy-preview-2145--redux-starter-kit-docs.netlify.app/api/configureStore#preloadedstate

appState: AppState
}

// Not correct, we don't provide a value for `theme`. You would get the following typescript error if you tried to assign preloadedState the `PreloadedState<RootState>` type: Property 'theme' is missing in type '{ showNavigationPanel: false; }' but required in type '{ theme: "Dark" | "Light"; showNavigationPanel: boolean; }'.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✋ This line should be broken into multiple separate // comments, so that there's no horizontal scrolling needed


> **Note**: `preloadedState` is not recursively partial! For each part of your application's initial state that you decide to provide initial values for, you must define values for **all** properties of that part of state. They will be passed to your reducers as the initial state, and they themselves can't be partial.

Let's discuss the above point some more and provide a few tips for how you fully leverage `preloadedState` in your application and tests. For example, let's stay we start with the following initial state:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✋ The info from here on down is useful, but I don't think it needs to be part of the main flow of the page.

Over in the Redux core docs, we have a <DetailedExplanation> component that toggles display of content, as soon at https://redux.js.org/style-guide/#structure-files-as-feature-folders-with-single-file-logic and other spots.

We don't have that component in this repo, but it should be straightforward to copy-paste it over and wrap that around all the content after the "Note" block.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! I'll look into adding that. Thank you

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