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

Revise Persisting store data doc page #1497

Merged

Conversation

sewera
Copy link
Collaborator

@sewera sewera commented Dec 29, 2022

Related Issues

Relates to #1220.

Summary

  • add cross-links
  • add line breaks for easier Markdown reading
  • unify Persist middleware spelling
  • change fishes to bears, because the plural of fish is fish,
    and it would be less readable
  • make sentences more concise

Check List

  • yarn run prettier for formatting code and docs

- add cross-links
- add line breaks for easier Markdown reading
- unify Persist middleware spelling
- change fishes to bears, because the plural of fish is fish,
  and it would be less readable
- make sentences more concise
@codesandbox-ci
Copy link

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 e310f19:

Sandbox Source
React Configuration
React Typescript Configuration
React Browserify Configuration
React Snowpack Configuration
React Parcel Configuration
Next.js Configuration

The given name is going to be the key used to store your Zustand state in the storage, so it must be unique.
The given name is going to be the key
used to store your Zustand state in the storage,
so it must be unique.

### `getStorage`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I found getStorage name misleading.
When I set up the Persist middleware options,
I intuitively wanted to set the storage with a storage option.
The getStorage option suggests that it doesn't set anything,
but simply returns the currently set storage.

I didn't want to change the code, but if it's possible,
I'd suggest changing getStorage to storage.

Copy link
Member

Choose a reason for hiding this comment

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

Naming getStorage means a function that returns a storage.

Anyway, #1463 will give a new option.

persist(
(set, get) => ({
fishes: 0,
addAFish: () => set({ fishes: get().fishes + 1 }),
bears: 0,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I changed fishes to bears, because the most used
plural form of fish is "fish". Bears not only will
be less confusing in that regard, but also will
play nicely with the Zustand branding.

Copy link
Collaborator

Choose a reason for hiding this comment

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

i like the idea of removing the ambiguity

Comment on lines -63 to -69
```ts
interface Storage {
getItem: (name: string) => string | null | Promise<string | null>
setItem: (name: string, value: string) => void | Promise<void>
removeItem: (name: string) => void | Promise<void>
}
```
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed the Storage interface definition,
because it introduced unnecessary duplication
with TypeScript definition in code.

### `serialize`

> Schema: `(state: Object) => string | Promise<string>`
> Type: `(state: Object) => string | Promise<string>`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I replaced "Schema" with "Type", because a schema reminds me more
of an API schema in remote communication, like REST or GraphQL schema.

@@ -312,9 +334,10 @@ useBoundStore.persist.getOptions().name

### `setOptions`

> Schema: `(newOptions: PersistOptions) => void`
> Type: `(newOptions: Partial<PersistOptions>) => void`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I aligned the type with persist.ts@77.

@@ -395,29 +424,49 @@ unsub()

## Hydration and asynchronous storages

To explain what's the "cost" of asynchronous storages, you need to understand what's hydration.
In a nutshell, hydration is the process of retrieving the persisted state from the storage and merging it with the current state.
To explain what is the "cost" of asynchronous storages,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I introduced Semantic line breaks
for better maintainability of those paragraphs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That is a really interesting read! Thank you for sharing (and introducing the change)

@sewera sewera changed the title Documentation update individual page audit Revise Persisting store data doc page Dec 29, 2022
Copy link
Collaborator

@chrisk-7777 chrisk-7777 left a comment

Choose a reason for hiding this comment

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

extremely well written, thank you for the changes

persist(
(set, get) => ({
fishes: 0,
addAFish: () => set({ fishes: get().fishes + 1 }),
bears: 0,
Copy link
Collaborator

Choose a reason for hiding this comment

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

i like the idea of removing the ambiguity

@@ -395,29 +424,49 @@ unsub()

## Hydration and asynchronous storages

To explain what's the "cost" of asynchronous storages, you need to understand what's hydration.
In a nutshell, hydration is the process of retrieving the persisted state from the storage and merging it with the current state.
To explain what is the "cost" of asynchronous storages,
Copy link
Collaborator

Choose a reason for hiding this comment

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

That is a really interesting read! Thank you for sharing (and introducing the change)

Copy link
Member

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

The given name is going to be the key used to store your Zustand state in the storage, so it must be unique.
The given name is going to be the key
used to store your Zustand state in the storage,
so it must be unique.

### `getStorage`
Copy link
Member

Choose a reason for hiding this comment

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

Naming getStorage means a function that returns a storage.

Anyway, #1463 will give a new option.

@dai-shi dai-shi merged commit a8c98d6 into pmndrs:main Dec 30, 2022
@sewera sewera deleted the documentation-update-individual-page-audit branch December 30, 2022 23:10
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.

None yet

3 participants