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

useLocalSearchParams should not use Partial for params #28813

Closed
rodrigorm opened this issue May 13, 2024 · 5 comments
Closed

useLocalSearchParams should not use Partial for params #28813

rodrigorm opened this issue May 13, 2024 · 5 comments
Assignees
Labels
needs review Issue is ready to be reviewed by a maintainer Router expo-router

Comments

@rodrigorm
Copy link
Contributor

Minimal reproducible example

Does not have yet

Which package manager are you using? (Yarn is recommended)

yarn

If the issue is web-related, please select the bundler (web.bundler in the app.json)

None

Summary

When using useLocalSearchParams, I am expecting to type the params according to its usage. For an route file like app/[teamId]/(root)/(pickups)/pickups/[id]/index.tsx, whe using the hook in typescript like below, I should expect teamId and id to be required and not optional as it is required in the route path, but the usage of Partial at [https://github.com/expo/expo/blob/main/packages/expo-router/src/hooks.ts#L102] forces me to use the non-null assertion ! on every variable usage inside the page.

const {
    id,
    teamId,
    status = "",
    type = "",
    position = "",
  } = useLocalSearchParams<{
    id: string;
    teamId: string;
    status?: string;
    type?: string;
    position?: string;
  }>();

Environment

expo-env-info 1.2.0 environment info:
System:
OS: Linux 5.15 Ubuntu 22.04.4 LTS 22.04.4 LTS (Jammy Jellyfish)
Shell: 5.1.16 - /bin/bash
Binaries:
Node: 18.17.1 - /usr/bin/node
Yarn: 1.22.19 - /usr/bin/yarn
npm: 9.6.7 - /usr/bin/npm
npmPackages:
expo: ~51.0.4 => 51.0.4
expo-router: ~3.5.12 => 3.5.12
react: 18.2.0 => 18.2.0
react-dom: 18.2.0 => 18.2.0
react-native: 0.74.1 => 0.74.1
react-native-web: ~0.19.11 => 0.19.11
npmGlobalPackages:
eas-cli: 9.0.5
Expo Workflow: managed

@rodrigorm rodrigorm added needs validation Issue needs to be validated Router expo-router labels May 13, 2024
@expo-bot expo-bot added needs review Issue is ready to be reviewed by a maintainer and removed needs validation Issue needs to be validated labels May 13, 2024
@marklawlor
Copy link
Contributor

This is a limitation of TypeScript. TypeScript is not "file" aware, it doesn't know (or care) where the types are being used. The best "fix" to this I've seen is SveltKit, but that adds a layer of complexity I'm not sure that we want (e.g importing from a "magical" path instead of the module)

I'm going to close this as at this current time, we're not going to fix this - but that may change in the future

@rodrigorm
Copy link
Contributor Author

rodrigorm commented May 17, 2024

Sorry, I dont get it. Why not just remove Partial<> from the useLocalSearchParams definition and allow developer to set its types?

@elfennani
Copy link

I upgraded expo-router, and now I have tons of files to "fix". I don't understand why you used Partial?

@simoncubera
Copy link

I agree with @rodrigorm and @elfennani, it's not clear to me why the Partial<> type-definition cannot be removed. Removing it would still allow to define optional parameters by explicitly specifying them as optional parameters using the ? syntax. Or is there anything we're overlooking? Is there a chance this issue can be re-opened @marklawlor?

@kyliau
Copy link

kyliau commented Jun 7, 2024

Running into this issue, and I think it's worth a second look.
To be clear, we are referring to the type signature of useLocalSearchParams:

export function useLocalSearchParams<
TParams extends SearchParams = SearchParams,
>(): Partial<TParams> {

Since it's a generic, it should be up to the developer to decide whether a property is optional, and not make the entire object Partial.
In fact, according to expo-router docs:

When the app/[user] route is matched, the user parameter is passed to the component and never a nullish value.

The Partial signature contradicts the statement above.

kyliau pushed a commit to kyliau/expo that referenced this issue Jun 7, 2024
`useLocalSearchParams` accepts a generic that's defined by the user,
so it should let user decide what's optional and not make the entire
return type `Partial`.

In fact, according to the official [docs](https://docs.expo.dev/router/reference/search-parameters/#route-parameters-versus-search-parameters)
> When the app/[user] route is matched, the user parameter is passed to the component and never a nullish value.

The use of `Partial` contradicts the statement above.

Issue expo#28813
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Issue is ready to be reviewed by a maintainer Router expo-router
Projects
None yet
Development

No branches or pull requests

6 participants