-
-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Typesafety improvements #12019
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
Merged
Merged
Typesafety improvements #12019
Changes from 29 commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
7ce143e
typescript language service plugin setup
pcattori ff7631d
simplify rollup config with `output.exports: "auto"`
pcattori fbcc3ad
vite node context refactor
pcattori c5aa105
typegen watch
pcattori eed704c
params typegen
pcattori 7d1a623
update playground for typegen
pcattori ab5e307
refactor
pcattori 6440d23
typegen exports
pcattori 6245f09
type tests
pcattori 624093f
decision doc: type inference
pcattori 76ef8a1
pr feedback
pcattori 60adb8c
arg-centric typegen
pcattori 7de6935
use types from typegen in compiler template
pcattori a447ee0
typegen command
pcattori 55fd1bf
update playgrounds to fix typechecking
pcattori 3bba333
update cli snapshots
pcattori 72a20c3
Fix vite-node resolution within TS plugin
markdalgleish 722db5c
Merge pull request #12039 from remix-run/markdalgleish/fix-ts-plugin-…
pcattori 50ba6c5
rename type utils to avoid autoimporting them within route modules
pcattori bdfefbe
typegen file comments
pcattori 12b2606
only typegen if route file exists
pcattori 07f9b5e
Merge branch 'dev' into pedro/typesafety-phase-1
pcattori 197c225
clientLoader.hydrate types
pcattori 38a2037
typegen as `.d.ts` instead of `.ts`
pcattori 23c16a6
typegen for root route
pcattori 1dd6884
inline ctx for ts plugin
pcattori dce0795
no more hardcoded app dir
pcattori f042b8c
decision doc rejected solutions section
pcattori 92d4a70
fix HydrateFallback detection
pcattori 42a5853
rename `DefaultProps` to `ComponentProps`
pcattori 6eaf2d6
pr feedback
pcattori 7e87c5d
changeset
pcattori File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"typescript.tsdk": "node_modules/typescript/lib" | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ | |
|
||
Date: 2022-07-11 | ||
|
||
Status: accepted | ||
Status: Superceded by [#0012](./0012-type-inference.md) | ||
|
||
## Context | ||
|
||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,270 @@ | ||
# Type inference | ||
|
||
Date: 2024-09-20 | ||
|
||
Status: accepted | ||
|
||
Supercedes [#0003](./0003-infer-types-for-useloaderdata-and-useactiondata-from-loader-and-action-via-generics.md) | ||
pcattori marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Context | ||
|
||
Now that Remix is being merged upstream into React Router, we have an opportunity to revisit our approach to typesafety. | ||
|
||
### Type inference | ||
|
||
There are three major aspects to typesafety in a framework like React Router: | ||
|
||
1. **Type inference from the route config** | ||
|
||
Some types are defined in the route config (`routes.ts`) but need to be inferred within a route module. | ||
|
||
For example, let's look at URL path parameters. | ||
Remix had no mechanism for inferring path parameters as that information is not present _within_ a route module. | ||
If a route's URL path was `/products/:id`, you'd have to manually specify `"id"` as a valid path parameter within that route module: | ||
|
||
```ts | ||
const params = useParams<"id">(); | ||
params.id; | ||
``` | ||
|
||
This generic was nothing more than a convenient way to do a type cast. | ||
You could completely alter the URL path for a route module, typechecking would pass, but then you would get runtime errors. | ||
|
||
2. **Type inference within a route** | ||
|
||
Some types are defined within a route module but need to be inferred across route exports. | ||
|
||
For example, loader data is defined by the return type of `loader` but needs to be accessed within the `default` component export: | ||
|
||
```ts | ||
export function loader() { | ||
// define here 👇 | ||
return { planet: "world" }; | ||
} | ||
|
||
export default function Component() { | ||
// access here 👇 | ||
const data = useLoaderData<typeof loader>(); | ||
} | ||
``` | ||
|
||
Unlike the `useParams` generic, this isn't just a type cast. | ||
The `useLoaderData` generic ensures that types account for serialization across the network. | ||
However, it still requires you to add `typeof loader` every time. | ||
|
||
Not only that, but complex routes get very tricky to type correctly. | ||
|
||
> What if the route also has a `clientLoader`? | ||
> Did you remember to change the generic to `typeof clientLoader`? | ||
> But in the initial SSR render, the `clientLoader` doesn't run, so really you'd need `typeof loader | typeof clientLoader`. | ||
> Unless you also set `clientLoader.hydrate = true` _AND_ provided a `HydrateFallback`. Then `clientLoader` always runs so you'd want just `typeof clientLoader` | ||
pcattori marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
The generic for `useLoaderData` starts to feel a lot like doing your taxes: there's only one right answer, Remix knows what it is, but you're going to get quizzed on it anyway. | ||
|
||
3. **Type inference across routes** | ||
|
||
Some types are defined in one route module but need to be inferred in another route module. | ||
This is common when wanting to access loader data of matched routes like when using `useMatches` or `useRouteLoaderData`. | ||
|
||
```ts | ||
import type { loader as otherLoader } from "../other-route.ts"; | ||
// hope the other route is also matched 👇 otherwise this will error at runtime | ||
const otherData = useRouteLoaderData<typeof otherLoader>(); | ||
``` | ||
|
||
Again, its up to you to wire up the generics with correct types. | ||
In this case you need to know both types defined in the route config (to know which routes are matched) and types defined in other route modules (to know the loader data for those routes). | ||
|
||
In practice, Remix's generics work fine most of the time. | ||
But they are mostly boilerplate and can become error-prone as the app scales. | ||
An ideal solution would infer types correctly on your behalf, doing away with tedious generics. | ||
|
||
## Goals | ||
|
||
- Type inference from the route config (`routes.ts`) | ||
- Type inference within a route | ||
- Type inference across routes | ||
- Same code path for type inference whether using programmatic routing or file-based routing | ||
- Compatibility with standard tooling for treeshaking, HMR, etc. | ||
- Minimal impact on runtime API design | ||
|
||
## Decisions | ||
|
||
### Route exports API | ||
|
||
Keep the route module export API as is. | ||
Route modules should continue to export separate values for `loader`, `clientLoader`, `action`, `ErrorBoundary`, `default` component, etc. | ||
That way standard transforms like treeshaking and React Fast Refresh (HMR) work out-of-the-box. | ||
|
||
Additionally, this approach introduces no breaking changes allowing Remix users to upgrade to React Router v7 more easily. | ||
|
||
### Pass path params, loader data, and action data as props | ||
|
||
Hooks like `useParams`, `useLoaderData`, and `useActionData` are defined once in `react-router` and are meant to be used in _any_ route. | ||
Without any coupling to a specific route, inferring route-specific types becomes impossible and would necessitate user-supplied generics. | ||
|
||
Instead, each route export should be provided route-specific args: | ||
|
||
```ts | ||
// Imagine that we *somehow* had route-specific types for: | ||
// - LoaderArgs | ||
// - ClientLoaderArgs | ||
// - DefaultProps | ||
|
||
export function loader({ params }: LoaderArgs) {} | ||
|
||
export function clientLoader({ params, serverLoader }: ClientLoaderArgs) {} | ||
|
||
export default function Component({ | ||
params, | ||
loaderData, | ||
actionData, | ||
}: DefaultProps) { | ||
// ... | ||
} | ||
``` | ||
|
||
We'll keep those hooks around for backwards compatibility, but eventually the aim is to deprecate and remove them. | ||
We can design new, typesafe alternatives for any edge cases. | ||
|
||
### Typegen | ||
|
||
While React Router will default to programmatic routing, it can easily be configured for file-based routing. | ||
That means that sometimes route URLs will only be represented as file paths. | ||
Unfortunately, TypeScript cannot use the filesystem as part of its type inference nor type checking. | ||
The only tenable way to infer types based on file paths is through code generation. | ||
|
||
We _could_ have typegen just for file-based routing, but then we'd need to maintain a separate code path for type inference in programmatic routing. | ||
To keep things simple, React Router treats any value returned by `routes.ts` the same; it will not make assumptions about _how_ those routes were constructed and will run typegen in all cases. | ||
|
||
To that end, React Router will generate types for each route module into a special, gitignored `.react-router` directory. | ||
For example: | ||
|
||
```txt | ||
- .react-router/ | ||
- types/ | ||
- app/ | ||
- routes/ | ||
- +types.product.ts | ||
- app/ | ||
- routes/ | ||
- product.tsx | ||
``` | ||
|
||
The path within `.react-router/types` purposefully mirrors the path to the correspond route module. | ||
pcattori marked this conversation as resolved.
Show resolved
Hide resolved
|
||
By setting things up like this, we can use `tsconfig.json`'s [rootDirs](https://www.typescriptlang.org/tsconfig/#rootDirs) option to let you conveniently import from the typegen file as if it was a sibling: | ||
|
||
```ts | ||
// app/routes/product.tsx | ||
import { LoaderArgs, DefaultProps } from "./+types.product"; | ||
``` | ||
|
||
TypeScript will even give you import autocompletion for the typegen file and the `+` prefix helps to distinguish it as a special file. | ||
Big thanks to Svelte Kit for showing us that [`rootDirs` trick](https://svelte.dev/blog/zero-config-type-safety#virtual-files)! | ||
|
||
### TypeScript plugin | ||
|
||
Typegen solutions often receive criticism due to typegen'd files becoming out of sync during development. | ||
This happens because many typegen solutions require you to then rerun a script to update the typegen'd files. | ||
|
||
Instead, our typegen will automatically run within a TypeScript plugin. | ||
That means you should never need to manually run a typegen command during development. | ||
It also means that you don't need to run our dev server for typegen to take effect. | ||
The only requirement is that your editor is open. | ||
pcattori marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Additionally, TypeScript plugins work with any LSP-compatible editor. | ||
That means that this single plugin will work in VS Code, Neovim, or any other popular editor. | ||
|
||
Even more exciting is that a TS plugin sets the stage for tons of other DX goodies: | ||
|
||
- jsdoc and links to official documentation when you hover a route export | ||
- Snippet-like autocomplete for route exports | ||
- In-editor warnings when you forget to name your React components, which would cause HMR to fail | ||
- ...and more... | ||
|
||
## Rejected solutions | ||
|
||
### `defineRoute` | ||
|
||
Early on, we considered changing the route module API from many exports to a single `defineRoute` export: | ||
|
||
```tsx | ||
export default defineRoute({ | ||
loader() { | ||
return { planet: "world" }; | ||
}, | ||
Component({ loaderData }) { | ||
return <h1>Hello, {loaderData.planet}!</h1>; | ||
}, | ||
}); | ||
``` | ||
|
||
That way `defineRoute` could do some TypeScript magic to infer `loaderData` based on `loader` (type inference within a route). | ||
With some more work, we envisioned that `defineRoute` could return utilities like a typesafe `useRouteLoaderData` (type inference across routes). | ||
|
||
However, there were still many drawbacks with this design: | ||
|
||
1. Type inference across function arguments depends on the ordering of those arguments. | ||
That means that if you put `Component` before `loader` type inference is busted and you'll get gnarly type errors. | ||
|
||
2. Any mechanism expressible solely as code in a route module cannot infer types from the route config (`routes.ts`). | ||
That means no type inference for things like path params nor for `<Link to="..." />`. | ||
|
||
3. Transforms that expect to operate on module exports can no longer access parts of the route. | ||
For example, bundlers would only see one big export so they would bail out of treeshaking route modules. | ||
Similarly, React-based HMR via React Fast Refresh looks for React components as exports of a module. | ||
It would be possible to augment React component detection for HMR to look within a function call like `defineRoute`, but it significantly ups the complexity. | ||
|
||
### `defineLoader` and friends | ||
|
||
Instead of a single `defineRoute` function as described above, we could have a `define*` function for each route export: | ||
|
||
```tsx | ||
import { defineLoader } from "./+types.product"; | ||
|
||
export const loader = defineLoader(() => { | ||
return { planet: "world" }; | ||
}); | ||
``` | ||
|
||
That would address the most of the drawbacks of the `defineRoute` approach. | ||
However, this adds significant noise to the code. | ||
It also means we're introducing a runtime API that only exists for typesafety. | ||
|
||
Additionally, utilities like `defineLoader` are implemented with an `extends` generic that [does not pin point incorrect return statements](https://tsplay.dev/WJP7ZN): | ||
|
||
```ts | ||
const defineLoader = <T extends Loader>(loader: T): T => loader; | ||
|
||
export const loader = defineLoader(() => { | ||
// ^^^^^^^ | ||
// Argument of type '() => "string" | 1' is not assignable to parameter of type 'Loader'. | ||
// Type 'string | number' is not assignable to type 'number'. | ||
// Type 'string' is not assignable to type 'number'.(2345) | ||
|
||
if (Math.random() > 0.5) return "string"; // 👈 don't you wish the error was here instead? | ||
return 1; | ||
}); | ||
``` | ||
|
||
### Zero-effort typesafety | ||
|
||
Svelte Kit has a ["zero-effort" type safety approach](https://svelte.dev/blog/zero-config-type-safety) that uses a TypeScript language service plugin to automatically inject types for framework-specific exports. | ||
Initially, this seemed like a good fit for React Router too, but we ran into a couple drawbacks: | ||
|
||
1. Tools like `typescript-eslint` that need to statically inspect the types of your TS files without running a language server would not be aware of the injected types. | ||
There's an open issue for [`typescript-eslint` interop with Svelte Kit](https://github.com/sveltejs/language-tools/issues/2073) | ||
|
||
2. Running `tsc` would perform typechecking without any knowledge of our custom language service. | ||
To fix this, we would need to wrap `tsc` in our own CLI that programmatically calls the TS typechecker. | ||
For Svelte Kit, this isn't as big of an issue since they already need their own typecheck command for the Svelte language: `svelte-check`. | ||
But since React Router is pure TypeScript, it would be more natural to invoke `tsc` directly in your `package.json` scripts. | ||
|
||
## Summary | ||
|
||
By leaning into automated typegen within a TypeScript plugin, we radically simplify React Router's runtime APIs while providing strong type inference across the entire framework. | ||
We can continue to support programmatic routing _and_ file-based routing in `routes.ts` while providing typesafety with the same approach and same code path. | ||
We can design our runtime APIs without introducing bespoke ways to inform TypeScript of the route hierarchy. | ||
|
||
The initial implementation will be focused on typesafety for path params, loader data, and action data. | ||
That said, this foundation lets us add type inference for things like `<Link to="..." />` and search params in the future. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.