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 note about using set() third param for logging #712

Merged
merged 3 commits into from
Dec 23, 2021

Conversation

omidantilong
Copy link
Contributor

Newcomers to zustand may be confused why devtools doesn't show action types by default (I was!), so I've added a note to the readme following this comment from @devanshj

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 13, 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 523af89:

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

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.

Looks good to me.
@devanshj can you double check please?

readme.md Outdated
devtools will only log actions from each separated store unlike in a typical *combined reducers* redux store. See an approach to combining stores https://github.com/pmndrs/zustand/issues/163

Because of this, actions will be logged as "anonymous" by default. To set a default log action type for all actions from your store: `devtools(store, {name: 'MyStore', anonymousActionType: 'MyStoreAction'})`
Copy link
Member

Choose a reason for hiding this comment

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

This seems an overlap with L442 example. Just a little bit though. If you have an idea to make the docs compact, it's welcomed.

readme.md Outdated
devtools will only log actions from each separated store unlike in a typical *combined reducers* redux store. See an approach to combining stores https://github.com/pmndrs/zustand/issues/163

Because of this, actions will be logged as "anonymous" by default. To set a default log action type for all actions from your store: `devtools(store, {name: 'MyStore', anonymousActionType: 'MyStoreAction'})`
Copy link
Contributor

Choose a reason for hiding this comment

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

The recommended approach is to use the third parameter instead of anonymousActionType so let's mention that first.

And then after that let's just write "If the third parameter is not passed, the action type by default becomes "anonymous". You can customize this via the anonymousActionType devtools option. Eg devtool(..., { anonymousActionType: 'unknown', ... })"

It doesn't make sense to use or encourage the store name as anonymousActionType because you can already see the store instance in top right drop-down.

readme.md Outdated
devtools will only log actions from each separated store unlike in a typical *combined reducers* redux store. See an approach to combining stores https://github.com/pmndrs/zustand/issues/163

Because of this, actions will be logged as "anonymous" by default. To set a default log action type for all actions from your store: `devtools(store, {name: 'MyStore', anonymousActionType: 'MyStoreAction'})`

Alternatively, `set` functions take a third parameter which can be used to log any action type you choose:
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we're writing this first, let's write this: "You can log an action for each set by passing an action type to the third parameter. Example:"

@devanshj
Copy link
Contributor

With the suggested changes, lgtm.

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.

We are waiting for some changes, right?

@devanshj
Copy link
Contributor

We are waiting for some changes, right?

Yep

@omidantilong
Copy link
Contributor Author

Hey, sorry, I didn't realise you were waiting for me! I will try and make the suggested changes over xmas when I have some free time. I've been a bit busy the last week or so.

@omidantilong
Copy link
Contributor Author

Found the free time :)

Copy link
Contributor

@devanshj devanshj left a comment

Choose a reason for hiding this comment

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

Thanks! :) Lgtm.

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! Merging.

@dai-shi dai-shi merged commit 0aeaf7f into pmndrs:main Dec 23, 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.

3 participants