dai-shi / react-hooks-global-state

[NOT MAINTAINED] Simple global state for React with Hooks API without Context API
https://www.npmjs.com/package/react-hooks-global-state
MIT License
1.1k stars 62 forks source link

allow undefined initial state in createStore #9

Closed jarlah closed 5 years ago

jarlah commented 5 years ago

This Is related to issue #8

Having to be forced to supply initial state breaks my flow ;)

jarlah commented 5 years ago

It does support it if i assign a default action in my reducer with my undefined type. I think a reducer should be self contained and not depend on any arguments.

jarlah commented 5 years ago

well combineReducers from redux does not support it, and creates a reducer that requires state and action. But if I can use something else, a custom combineReducers, that does not require state and action, then may code will compile. Until then ill just suppress it. Its my problem. But your library doesn't care about this problem, since its pure js.

jarlah commented 5 years ago

my reducer looks like this now, and should support this fix:

export function searchReducer(state: SearchState = initialState, action: SearchAction = { type: undefined } as any) {
    switch (action.type) {
        case getType(SearchActions.search.request): return {
            ...state,
            loading: true,
            hits: []
        };
        case getType(SearchActions.search.success): return {
            ...state,
            loading: false,
            hits: action.payload
        };
        case getType(SearchActions.search.failure): return {
            ...state,
            loading: false,
            error: action.payload
        };
        default: return state;
    }
}

its a liiiitle bit hacky with the any cast, but it should just fall through to default case and everything good.

dai-shi commented 5 years ago

Thanks. Let me merge this.

dai-shi commented 5 years ago

Hmm, I realized it's a bit hacky too. I should probably define the Reducer type as

export type Reducer<S, A> = (state?: S, action?: A) => S;

But, this is a breaking change.

jarlah commented 5 years ago

Well. Actually. When i think about it. Lets say there is an existing project using this lib. Where it doesnt have default value for action defined. Well. They will most propably follow old style and supply initial state. And everything is good. So, i am not sure if its breaking old project. But it changes the expected behavior and docs must be updated.

jarlah commented 5 years ago

I think however its best to do reducer(undefined, { type: undefined } } as was my first example. Then you can say that action is optional but its not required to set default value.

jarlah commented 5 years ago

You could also say that initial state is empty object. But that doesnt work too well for simple reducers with piimitive. If you dont supply initial state then it should be required by the reducer to set initial state

jarlah commented 5 years ago

at least thats how react and redux works. You dont have to supply default value for action and react redux will pass inn an initial action when it initializes the store.

dai-shi commented 5 years ago

Hi, I changed my mind a bit. Reverted to your suggestion: initialState = reducer(undefined, { type: undefined });. However, initialState is not "optional", and you can only pass undefined if your reducer accepts it. Please let me know how you like it.

The other concern is whether this breaks devtools or not. You might want to try that.