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

No type safety on useSearch({ strict: false }) #1217

Open
RobertOstermann opened this issue Feb 24, 2024 · 16 comments · Fixed by #1224
Open

No type safety on useSearch({ strict: false }) #1217

RobertOstermann opened this issue Feb 24, 2024 · 16 comments · Fixed by #1224

Comments

@RobertOstermann
Copy link

Describe the bug

The type safety is not working as expected for useSearch or useParams when using strict: false in the options.

Your Example Website or App

https://stackblitz.com/edit/tanstack-router-nan1d9?file=src%2Froutes%2Fdashboard.users.tsx

Steps to Reproduce the Bug or Issue

  1. Go to the Kitchen Sink File Base Example (any example with search params would work)
  2. Change the route.useSearch to useSearch({ strict: false })
  3. Add a useParams({ strict: false })
  4. The types are not showing for either function as expected

Expected behavior

With strict: false the type safety should be loosened to show all available types.

Screenshots or Videos

useSearch({ strict: false }) no type safety

image

useSearch({ strict: false, experimental_returnIntersection: true }) does not work. This does fix the same issue I see with useParams` though.

image

useParams({ strict: false }) no type safety

image

useParams({ strict: false, experimental_returnIntersection: true }) fixes the issue for useParams

image

Platform

tanstack router version: 1.16.6
typescript: 5.3.3 (on my local, not sure what stackblitz is on)

Additional context

No response

@schiller-manuel
Copy link
Contributor

useParams({ strict: false }) no type safety

This is incorrect, this is type safe. You get back the union of all possible search parameters.

see https://stackblitz.com/edit/tanstack-router-kws5zj?file=src%2Froutes%2Fdashboard.users.tsx

const search: {} | {} | {} | {
    showNotes?: boolean | undefined;
    notes?: string | undefined;
} | {
    usersView?: {
        sortBy?: "id" | "name" | "email" | undefined;
        filterBy?: string | undefined;
    } | undefined;
} | {
    ...;
} | {
    ...;
} | {
    ...;
}

useSearch({ strict: false, experimental_returnIntersection: true }) does not work.

Indeed, on stackblitz it does not work.
In VS Code it works as expected, so please try in a proper IDE:

Screenshot 2024-02-25 at 21 49 24

@RobertOstermann
Copy link
Author

@schiller-manuel I have tested in a "proper IDE". It was my thought that StackBlitz used VS Code on the web? So I figured providing StackBlitz examples would be easiest to reproduce. Here is what I am seeing on VS Code on my local machine...

useParams({ strict: false }): This does not work as I would expect in VS Code, or in StackBlitz. I would expect to get autocomplete when using this setup. It does return a union, as you have noted, but there is no autocomplete support and it views using any of the params as invalid.

image

useParams({ strict: false, experimental_returnIntersection: true });: Works as expected

image

useSearch({ strict: false }): This does not work as I would expect in VS Code. It has the same behavior as useParams({ strict: false }) and returns a union, but VS Code does not provider autocomplete support.

image

useSearch({ strict: false, experimental_returnIntersection: true }): This works as expected for a smaller amount of routes, but I get the Type instantiation is excessively deep and possibly infinite error on a project with ~30 routes, though only about 14 total search params.

image

@schiller-manuel
Copy link
Contributor

there is no autocomplete support

Since one of the union members is the empty object {}, TS cannot provide any autocomplete here if you don't narrow down the union since the union members don't have any properties in common (those would be the ones that could be autocompleted).

Can you please provide a reproducer where the Type instantiation is excessively deep and possibly infinite error occurs in a local VS Code instance?

@RobertOstermann
Copy link
Author

RobertOstermann commented Feb 25, 2024

@schiller-manuel Ok, that makes sense, so only by using the experimental_returnIntersection will I get the autocomplete support I was expecting. Thank you for the quick reply.

I was able to reproduce the Type instantiation error, here is an example on CodeSandbox. I cloned the repo and tested on my local to ensure it was not just an issue on CodeSandbox.

CodeSandbox

Line 435 has the error

@schiller-manuel
Copy link
Contributor

Did you maybe share the wrong link? there is no error on line 435

Screenshot 2024-02-25 at 23 22 02

@RobertOstermann
Copy link
Author

Oh my bad, forgot to fork it. Try this link

CodeSandbox

@schiller-manuel
Copy link
Contributor

schiller-manuel commented Feb 25, 2024

thanks for the reproducer, the fix is released with https://github.com/TanStack/router/releases/tag/v1.17.4

well... at least for some cases :(

@RobertOstermann
Copy link
Author

Hm yes, seems like it worked for my smaller example, but once I updated that to have a few more routes it unfortunately broke again.

@rajbirjohar
Copy link

Do you know what could be some possible usecases where useSearch would return an empty object as a type? I too am facing this issue and from as far as I can tell when using zod schemas atleast, it will give me the correct union of all the types but adds on that empty object.

@schiller-manuel
Copy link
Contributor

schiller-manuel commented May 16, 2024

you probably have at least one route without a validateSearch and thus the empty object will be put into the union

@rajbirjohar
Copy link

@schiller-manuel Gotcha, yes I have multiple. Workaround? Should we add a validateSearch to all routes and somehow return undefined?

@schiller-manuel
Copy link
Contributor

why is having the empty object a problem?

@rajbirjohar
Copy link

rajbirjohar commented May 16, 2024

Because I am getting a type safety issue where the key does not exist within the empty object.

const { page } = useSearch({
    strict: false
  });
Property 'page' does not exist on type '{} | { redirect?: string | undefined; } | { sort?: string | undefined; search?: string | undefined; page?: number | undefined; limit?: number | undefined; } | { sort?: string | undefined; search?: string | undefined; page?: number | undefined; limit?: number | undefined; } | { ...; }'.

Edit: Specifically this:

Property 'page' does not exist on type {}.

I have plenty of pages that are validating the search params including a page param but it still cannot detect it. Do I have to assert it as a certain shape because that would be sort of a pain to deal with.

The use case I have is a component that updates a single param in the url across multiple tables/pages.

@schiller-manuel
Copy link
Contributor

you also have a route with search { redirect?: string | undefined; }, this also does not have the page property.
So the empty object is not the only one that prevents you from destructuring like you showed.

If experimental_returnIntersection does not work for you, you need to narrow down the union yourself, e.g. like this:

const search = useSearch({
    strict: false
});
if ('page' in search) {
    console.log(page.search);
}

@rajbirjohar
Copy link

Got it. experimental_returnIntersection does work. Any idea when it will no longer be experimental? Also, should probably be another discussion but how would you handle reading the search params and converting them to a url param string like below? This doesn't work since it expects all values to be strings meaning I would need to coerce all numbers and join all string arrays before performing this. Is there a utility function available that would do this? I'd like to essentially extract the search params from the url then send them to my API as search params.

const searchParams = routeApi.useSearch();

const url = `/entity?${new URLSearchParams(searchParams).toString()}`;

@schiller-manuel
Copy link
Contributor

"Any idea when it will no longer be experimental?"
not really. The problem is that converting a union to an intersection relies on TypeScript hacks that are not stable (see the original issue with too many union members for example).

for other questions please ask in our discord community at https://discord.com/channels/719702312431386674/1023930177224462388

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