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
8.22k stars 648 forks source link

Generated router with TS errors using new `isolatedDeclarations` #1630

Open SimenB opened 6 months ago

SimenB commented 6 months ago

Describe the bug

Using isolatedDeclarations in the TypeScript 5.5 beta gives errors as it's not explicitly typed. Generating (and exporting) the type annotations would be nice.

https://devblogs.microsoft.com/typescript/announcing-typescript-5-5-beta/#isolated-declarations

Your Example Website or App

https://github.com/SimenB/router-isolated-declarations

Steps to Reproduce the Bug or Issue

Run the TypeScript compiler in the linked reproduction

Expected behavior

No type errors.

Screenshots or Videos

No response

Platform

N/A

Additional context

No response

SeanCassiere commented 6 months ago

@chorobin @schiller-manuel is isolatedDeclarations a flag we hope working with TSR?

Looking at the article linked, the solution to this seems to be the use of explicit types (and return types).

isolatedDeclarations should be adopted on a case-by-case basis.

They do also mention this though 👆🏼.

Should we be looking at this now? or should we wait for Typescript 5.5 to be released and then added into our type-matrix in CI?

SimenB commented 6 months ago

Note that this issue is specifically about the code generated by @tanstack/router-generator output into the user's code base, not necessarily the router's own code base.

(in theory, this might improve type checking performance as well as noted in https://typescript-eslint.io/rules/explicit-module-boundary-types/, but my point was mainly to help/not block users from using the flag)

SimenB commented 4 months ago

5.5 is out btw, and this issue is blocking me from adopting the new flag

schiller-manuel commented 4 months ago

how would the generated code need to change to support isolatedDeclarations?

SimenB commented 4 months ago

The exported value (i.e. routeTree) would need to be explicitly typed rather than inferred.

schiller-manuel commented 4 months ago

that's not possible AFAIK

SimenB commented 4 months ago

Why is that not possible? If it's able to be inferred, it should be possible to generated as well.

If nothing else, you can use tsc itself to output the explicit types.

image
SeanCassiere commented 4 months ago

@chorobin If we adjust the route-tree template output to infer from the global module declaration types, would this solve this?

SeanCassiere commented 2 months ago

@chorobin we recently spoke about exporting everything and about exporting everything from the roue-tree as well.

Would this be caught by tsc --noEmit? If so, could we add an example that uses this isolatedDeclarations option and have this be a part of our integration process?

chorobin commented 2 months ago

@chorobin we recently spoke about exporting everything and about exporting everything from the roue-tree as well.

Would this be caught by tsc --noEmit? If so, could we add an example that uses this isolatedDeclarations option and have this be a part of our integration process?

Do you mean using isolatedDeclarations as a way for us to catch these errors?

Currently I don't see users using this option as you'd need to annotate every route definition which of course we could generate and they could use but you lose out a lot on DX due to no inference and just everything requiring generation