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

createMatchSelector typedef string or object? #258

Closed
MitchWeiss opened this issue Feb 18, 2019 · 2 comments · Fixed by #432
Closed

createMatchSelector typedef string or object? #258

MitchWeiss opened this issue Feb 18, 2019 · 2 comments · Fixed by #432

Comments

@MitchWeiss
Copy link

There isn't much documentation on createMatchSelector but in other places eg #38, the parameter is an object:

const matchSelector = createMatchSelector({ path: '/course/:id })

The typedef added in #235 expects a string.

I believe the typedef is incorrect. My understanding of the code is that this parameter is provided as the second parameter in react-router's matchPath which expects an object https://github.com/ReactTraining/react-router/blob/master/packages/react-router/docs/api/matchPath.md

Can anyone else confirm that this typedef is incorrect?

@robchristian
Copy link

Would also like to see some documentation surface. The readme leaves me thinking this library doesn't have support for route params.

@zaverden
Copy link
Contributor

zaverden commented Jul 8, 2020

Current typedef does not cover all valid usecases.

export function createMatchSelector<
S extends RouterRootState, Params extends { [K in keyof Params]?: string }
>(path: string): matchSelectorFn<S, Params>;

The path argument is used a 2nd parameter for matchPah

const match = matchPath(pathname, path)

In the matchPah 2nd argument is defined as string | string[] | RouteProps: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/3e0007a3a88eca10f61fcb55d78dd66e323f24c6/types/react-router/index.d.ts#L133

supasate pushed a commit that referenced this issue Sep 13, 2020
`path` parameter should match parameter of the underlying `matchPath`

fix #258
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

Successfully merging a pull request may close this issue.

3 participants