TanStack / router

🤖 Fully typesafe Router for React (and friends) w/ built-in caching, 1st class search-param APIs, client-side cache integration and isomorphic rendering.
https://tanstack.com/router
MIT License
7.19k stars 486 forks source link

Route generation is adding a / at the end of routes (like $project/$tab/index.tsx) #1517

Closed divmgl closed 3 weeks ago

divmgl commented 3 weeks ago

Describe the bug

Say I have this folder structure:

Screenshot 2024-04-24 at 2 55 31 PM

The route generator is incorrectly adding a slash to the $tab/index route, breaking our app:

Screenshot 2024-04-24 at 2 56 11 PM

Without this slash the router works as expected but the generator continues to add a slash.

Your Example Website or App

It's our own internal app.

Steps to Reproduce the Bug or Issue

Say I have this folder structure:

Screenshot 2024-04-24 at 2 55 31 PM

The route generator is incorrectly adding a slash to the $tab/index route, breaking our app:

Screenshot 2024-04-24 at 2 56 11 PM

Without this slash the router works as expected but the generator continues to add a slash.

Expected behavior

A slash is not added.

Screenshots or Videos

No response

Platform

macOS Latest version of TanStack Router 1.30

Additional context

No response

divmgl commented 3 weeks ago

We were able to fix this by moving $tab/index.tsx to $tab.tsx. I'm guessing this is intentional?

divmgl commented 3 weeks ago

Just kidding that did not fix the issue. We now have a new issue:

        <Link
          to="/workspace/$workspaceId/projects/$projectId/$tab"
          params={{ projectId: projectId, tab: "overview" }}
        >
          {projectName}
        </Link>
Screenshot 2024-04-24 at 3 07 34 PM
divmgl commented 3 weeks ago

We had to remove TypeScript support for TanStack Router to unblock all of these issues:

// declare module "@tanstack/react-router" {
//  interface Register {
//    router: typeof router
//  }
// }
divmgl commented 3 weeks ago

Also your documentation website is down...

andrejilderda commented 3 weeks ago

I may be overseeing something, but didn't you mean to name your file route.tsx in stead of index.tsx?

Index routes specifically target their parent route when it is matched exactly and no child route is matched. We can see this in the above route tree with both the root index route (index.tsx) and the posts index route (posts.index.tsx). https://tanstack.com/router/latest/docs/framework/react/guide/routing-concepts#index-routes

schiller-manuel commented 3 weeks ago

what exactly is "breaking" your app?

please always provide a minimal complete example, e.g. by forking and modifying one of the existing examples on stackblitz

divmgl commented 3 weeks ago

@schiller-manuel https://github.com/divmgl/tanstack-router-bug

divmgl commented 3 weeks ago
Screenshot 2024-04-24 at 4 57 21 PM
divmgl commented 3 weeks ago
Screenshot 2024-04-24 at 4 57 35 PM
schiller-manuel commented 3 weeks ago

so does "breaking" mean typescript issues or runtime behavior?

divmgl commented 3 weeks ago

Changing this to index.tsx breaks the links because it adds a trailing slash, just a heads up. So when you click on the links the application goes to a Not Found route.

That's what I meant by "breaks the app".

divmgl commented 3 weeks ago

Both. TypeScript is not working correctly and a trailing slash will send us to a Not Found page on existing links if we use index.tsx.

divmgl commented 3 weeks ago

However the biggest blocker by far is that TypeScript is not working correctly.

schiller-manuel commented 3 weeks ago

can you please update your example to showcase the "trailing slash will send us to a Not Found page on existing links if we use index.tsx" issue? I can't reproduce it, but probably I don't understand the issue correctly.

divmgl commented 3 weeks ago

Will do. Are you able to see the TS issue though?

schiller-manuel commented 3 weeks ago

we are aware that a recent change that restricts linking to leaf routes only was too strict and this will be loosened up soon.

however, in your particular case, you might really need both index.tsx and route.tsx. route.tsx then contains all the layout that should be displayed for all children routes (e.g. /projects/123/456/activities/789), including the index route defined in index.tsx (e.g. /projects/123/456).

divmgl commented 3 weeks ago

Ah, very interesting. So we needed both an index.tsx and route.tsx file here. Kind of confusing, should definitely make a note about this in the documentation.

One additional question, is this intentional?

Screenshot 2024-04-24 at 5 21 10 PM

Where if $workspaceId and $projectId are not specified in the params then TypeScript complains? We found it really useful to be able to supply partial params and rely on @tanstack/react-router's functionality where it reuses the existing params using the current route.

schiller-manuel commented 3 weeks ago

For partial params to work, you need to be navigating away from a route that has those missing params defined (i.e. the from property needs to be set. Otherwise TS does not know in which route the link is created). However, as you already stated above there currently is a bug (https://github.com/TanStack/router/issues/1474) that prevents the params to be carried over at runtime.

schiller-manuel commented 3 weeks ago

Ah, very interesting. So we needed both an index.tsx and route.tsx file here. Kind of confusing, should definitely make a note about this in the documentation.

Did you have a look at https://tanstack.com/router/v1/docs/framework/react/guide/route-trees ? If this misses some information, we would really appreciate a PR to update the documentation.

divmgl commented 3 weeks ago

For partial params to work, you need to be navigating away from a route that has those missing params defined (i.e. the from property needs to be set. Otherwise TS does not know in which route the link is created). However, as you already stated above there currently is a bug (#1474) that prevents the params to be carried over at runtime.

So a previous version of TanStack Router did not work like this and we liked the DX quite a bit. The reason is that links are usually static and are only used in a single place. It'd be nice if this was opt-in or opt-out in some way. Maybe a feature request.

divmgl commented 3 weeks ago

Closing this issue as technically our problem here is resolved.

We're running into other issues with TanStack Router. One additional issue is performance in our TS Server is slowing down considerably as we iterate on routes. Here's a discussion I opened since I can't create a repro due to the size of our codebase https://github.com/TanStack/router/discussions/1518.

I'd be happy to coordinate with you all if you'd like to take a look at our codebase and be compensated for helping us. My email is dexter@permitflow.com.