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

Suggestion for modifying the use functions #50

Open
HIMISOCOOL opened this issue Aug 31, 2021 · 4 comments
Open

Suggestion for modifying the use functions #50

HIMISOCOOL opened this issue Aug 31, 2021 · 4 comments

Comments

@HIMISOCOOL
Copy link

You could change this line and all similar use functions from:

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

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
Copy link
Contributor

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
Copy link
Author

sure, I was thinking for all helpers.

@HIMISOCOOL
Copy link
Author

@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
Copy link
Contributor

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.

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

No branches or pull requests

2 participants