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

Remove confusing reset feature #44

Merged
merged 4 commits into from
Nov 14, 2021
Merged

Conversation

aralroca
Copy link
Collaborator

Fixes #41

Pre-released in 0.8.0-canary.1. (If you want to try it)

Remove the "reset" as the responsibility of the library. In this way:

Pros:

  • Removing confusion as to what exactly the reset function did. Simplifying and making it easier to understand.
  • Fewer things in memory. Thus it is not necessary to save the store + initialStore. Only the store. This improves performance for applications with very large stores.
  • Tiniest. Supporting reset made it take up more bits.
  • Removing a feature that did not add value to the library, since the same could be done with the setter.
  • More flexibility to the user to reset with the setter where the user wants: last value, last value fetched to an API, initial value...

Cons:

  • The user will be responsible for controlling the reset. That is, save the value in memory.

    Example:

    const [value, setValue] = useStore.value()
    const lastValue = useRef(value)
    
    // After some changes, is possible to reset with:
    setValue(lastValue.current)

    Another example:

     const initialStore = { count: 0 }
     export const { getStore } = createStore(initialStore)
     const [,setStore] = getStore()
    
     export const reset = () => setStore(initialStore)
     import { reset } from './store'
    
     function ResetStore() {
       return <button onClick={reset}>Reset store</button>
    }

@aralroca aralroca self-assigned this Nov 12, 2021
@aralroca aralroca added this to the 0.8.0 milestone Nov 12, 2021
Copy link
Collaborator

@danielart danielart left a comment

Choose a reason for hiding this comment

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

Could you change it on examples folder? I could manage that if you want in an other pr I changed this on examples folder

@aralroca
Copy link
Collaborator Author

An extra pro 🙂:

  • useStore proxy hook now is more familiar with React.useState.

@yathink3
Copy link

It make more sense now

@aralroca aralroca merged commit 1e98db5 into master Nov 14, 2021
@aralroca aralroca deleted the aral-remove-confusing-feature branch November 14, 2021 10:19
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.

Store not getting resetting properly
3 participants