greenpress / vuex-composition-helpers

A util package to use Vuex with Composition API easily.
https://www.npmjs.com/package/vuex-composition-helpers
MIT License
288 stars 32 forks source link

Suggestion for modifying the use functions #50

Open HIMISOCOOL opened 2 years ago

HIMISOCOOL commented 2 years ago

You could change this line and all similar use functions from: https://github.com/greenpress/vuex-composition-helpers/blob/1af7716cfa18e32313154d7ba39f5b457b3adfe9/src/namespaced.ts#L84 To be

useState: (...map: KnownKeys<TState>[]) => useNamespacedState(store, namespace as string, map),

which would eliminate the need to explicitly pass the map in as an array:

const { loading, error } = useState('loading', 'error');
davidmeirlevy commented 2 years ago

Interesting idea.

But you suggest it to work for inside the namespaced helper only?

Would you like to contribute this feature? Make sure it must have backwards comparability.

HIMISOCOOL commented 2 years ago

sure, I was thinking for all helpers.

HIMISOCOOL commented 2 years ago

@davidmeirlevy ok so I have a working prototype.

Using useState as an example

export function useState(storeOrMap, map) {
        let store = storeOrMap;

    if (isArray(store)) {
        map = store;
        store = getStoreFromInstance();
    }
    else if (isString(store)) {
        map = Array.from(arguments);
        store = getStoreFromInstance();
    }
    return useMapping(store, null, map, computedState);
}

This works in terms of javascript, however there is an issue with the typescript. method overloads get messy when you have rest parameters in your signatures

export function useState<TState>(map: KnownKeys<TState>[]): RefTypes<TState>
export function useState<TState>(...keys: KnownKeys<TState>[]): RefTypes<TState>
//                                ^^^^^^^^^^^^^^^^^^^^ `This overload signature is not compatible with its implementation signature.ts(2394)`
export function useState<TState>(store: Store<TState>, map: KnownKeys<TState>[]): RefTypes<TState>
export function useState<TState = any>(storeOrMap: Store<TState> | KnownKeys<TState>[], map?: KnownKeys<TState>[]): RefTypes<TState> {
 ...
}

...keys cant be accurately represented in the implementation signature (the bottom one) so it would never compile...

If the helpers could instead be partially applied (store) => (map) => { ... } or perhaps the useState where you provide store being a different variant it would be doable, but that would be a breaking change.

I have cleaned up some of the code changes I made in the process though and I can make a PR with those changes.

davidmeirlevy commented 2 years ago

Typescript integration is the hardest issue over here, I know. However, current users depend on it on their codebase, so we must make it work for both cases.

Maybe this function should have several different signatures in order to work. I would go for this type of solution.