-
Notifications
You must be signed in to change notification settings - Fork 228
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
useStore hook #733
Comments
@pablopalacios you want to take a look at this? |
@billyrrr yeah, I completely understand what you are saying. Currently There are only 2 use cases for it:
Creating the So, let's say, creating a very simple function useStore(storeName, getStateFromStore) {
const { getStore } = useFluxible().
const store = getStore(storeName);
const [state, setState] = useState(getStateFromStore(store));
function updateState() {
setState(getStateFromStore(store));
}
useEffect(() => {
store.on('change', updateState);
return () => store.removeListener('change', updateState);
}, [store]);
return state
} Notes:
However, as soon as we add But anyway, perhaps we could just start small and have no props support... @redonkulus what do you think? @billyrrr if you can provide a PR I will gladly support you with reviews ;) |
@pablopalacios Thank you for the detailed advice. I will start with the implementation of
If
@pablopalacios has mentioned that:
Is the lack of interface due to consistency issues? For example,
I will continue with a PR based on your Thanks! |
…eady) Co-authored-by: pablopalacios <[email protected]>
@billyrrr regarding using props inside Regarding the lacking of interface, I mean, it would be cool if const state = useStore('StoreName'); But since fluxible stores have no strict interface about how to retrieve their state, (eg. const getStateFromStore = store => {
return store.getState()
}
const Component = () => {
const state = useStore(Store, getStateFromStore);
// ...
} |
I realized that there were some issues with a solution I proposed earlier. (most due to shallow prop comparison, and will not work) (It has since been deleted. Will post an amended version soon. Please ignore commit e415fe7 which will be rolled back soon) |
@billyrrr I've created an example to demonstrate the stale props with the current |
Is your feature request related to a problem? Please describe.
useFluxible
hook is not as expressive asconnectToStore
.Describe the solution you'd like
const locationStore = useStore('UserGpsLocationStore')
which would trigger a rerender on user Gps location change. The Gps location value is used in multiple components (similar as context)
Before, the user's GPS location can be read from the context, but current implementation of fluxible-addons-react no longer supports legacy react Context API. It seems that the better way is to implement a
useStore
hook with a more fine-grained control over the data.Describe alternatives you've considered
1.
Using current
connectToStore
HOC solves the problem (but less elegantly).A wrapper has to be implemented on top of a hook component
connectToStore(<HookBasedComponent />, [StoreA, StoreB])
Downside:
Using current
useFluxible
hookI am not very clear on the behaviour of this hook (whether it would trigger a rerender on Store change). Either way, it seems that a more fine-grained hook would be better.
I am considering implementing this hook, but hope to get some advice and feedback from the community first.
The text was updated successfully, but these errors were encountered: