drcmda / react-contextual

🚀 react-contextual is a small (less than 1KB) helper around React 16s new context api
MIT License
642 stars 23 forks source link

Improve documentation in case of use of external store #13

Closed abenhamdine closed 6 years ago

abenhamdine commented 6 years ago

If I'm right, and according to my tests, two importants points are not clear in the docs :

1°) if an external store is used (ie declared with createStore), the prop to is required in the subscribe component, and we must pass the store :

Eg, following won't work with an external store

<Provider store={store}>
    <Subscribe select={(state) => state}>
        {(state: AppState) => {
            // TypeError : Cannot read property 'isLoggedIn' of undefined
            return state.login.isLoggedIn ? "Logged ?" : "Unlogged"
        }}
    </Subscribe>
</Provider>

whereas the following code works :

<Provider store={store}>
    <Subscribe to={store} select={(state) => state}>
        {(state: AppState) => {
            // ok, runs fine
            return state.login.isLoggedIn ? "Logged ?" : "Unlogged"
        }}
    </Subscribe>
</Provider>

Note that I passed store to the Subscribe component, and not ProviderContext as described in docs here : https://github.com/drcmda/react-contextual/blob/master/API.md#subscribe-as-a-component, so I'm confused what is the correct way to suscribe to an external store. So what is ProviderContext ? Should I import the store in every component I want to suscribe ?

2°) the external store must be passed to the provider with the store property :

eg :

<Provider store={store}>
        <Subscribe to={store} select={(state) => state}>
            {props => (
                <div>
                    <h1>{props.count}</h1>
                    <button onClick={props.actions.up}>Up</button>
                    <button onClick={props.actions.down}>Down</button>
                </div>
            )}
        </Subscribe>
    </Provider>

whereas with 'internal' store, store properties are spread :

<Provider {...store}>
        <Subscribe>
            {props => (
                <div>
                    <h1>{props.count}</h1>
                    <button onClick={props.actions.up}>Up</button>
                    <button onClick={props.actions.down}>Down</button>
                </div>
            )}
        </Subscribe>
    </Provider>

This should be emphasized in https://github.com/drcmda/react-contextual#external-store

drcmda commented 6 years ago

@abenhamdine good call! The internal store is more of a convenience thing, but i can see how it's confusing once you would use external stores or different context providers in general.

whereas with external store, store properties are spread :

As long as you have a property called store you can spread. The spreading stuf isn't technically part of the api, it's just easier sometimes to apply lots of values. If you have an external store you'd most likely do:

<Provider store={myStore} > ...

so I'm confused what is the correct way to suscribe to an external store. So what is ProviderContext ? Should I import the store in every component I want to suscribe ?

Correct, the internal store is the only exception, it behaves much like Redux where you only have one store and therefore you don't need to know any explicit contract.

But once you have external or multiple stores, or random React contexts, moduleContexts and so on, you need to know the provider, and you refer to it by reference as the first arg to subscribe or the to prop in Subscribe:

import store from '../assets/store'

<Subscribe to={store} select={store => ({ name: store.name })} />

// or ...

subscribe(store, store => ({ name: store.name })(Component)

It would be super helpful if you had any ideas how to word this properly. Going on a small vacation in a day and would be nice to have this in.

abenhamdine commented 6 years ago

Thx for you answer ! Actually, I have edited my initial post since I messed (sorry for that) the code of the two examples (external vs internal store) and my comment about spread properties was related to internal store, because in the docs (https://github.com/drcmda/react-contextual#render-props), I see :

// see the spread here :
<Provider {...store}>

whereas for external store, I see in the docs :

<Provider store={store}>

Otherwise, one point is still not totally clear for me : IIUC ProviderContext (see https://github.com/drcmda/react-contextual/blob/master/API.md#subscribemapcontexttopropsanycomponent) references the internal store. And if there is an internal store, first arg of Subscribe can be omitted. So is there a need for ProviderContext ?

It would be super helpful if you had any ideas how to word this properly.

I can try a PR. It would be usefull for newcomers to have this kind of explanations !

drcmda commented 6 years ago

You never need ProviderContext, it's the internal context used for the default store. If you skip the first arg and just give it the selector (or nothing at all), it will fetch ProviderContext. Though yes you could also use it if you wanted to.

I thought that it would be convenient like that and familiar to redux users, but it made setting the rules and testing quite intricate (https://github.com/drcmda/react-contextual/blob/master/tests/subscribe.js). It works now, but if it's confusing i'm totally open for suggestions.

drcmda commented 6 years ago

Updated both the docs and dependencies. React-broadcast is out.