-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
docs(tutorials/blog): add TS 4.9's satisfies
in combination with LoaderFunction
#4675
Conversation
|
Hi @tom-sherman, Welcome, and thank you for contributing to Remix! Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once. You may review the CLA and sign it by adding your name to contributors.yml. Once the CLA is signed, the If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run. Thanks! - The Remix team |
Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳 |
1e7397c
to
fd416d8
Compare
Ok so the Typescript change can be reverted, I got confused because the version of remix I have doesn't have a LoaderFunction type. @brophdawg11 do you think the docs change is valuable at all? |
yeah I think pointing out how |
Question, can you use |
@tom-sherman actually, since you can technically return anything ( |
@sergiodxa Unfortunately not, it appears the syntax doesn't support it right now - you can only use @brophdawg11 Oh you're absolutely right 🤦 in which case I don't think this even deserves a place in the docs. It might make sense as a more editorialised blog post I guess (I'm already writing one on my own blog) |
@sergiodxa A workaround is something like this: type Fn = () => number;
function foo() { return "ds" }
foo satisfies Fn; // type error Not recommended tho because compilers will likely output a |
One (function hello() {
return "world"
}) satisfies () => number; |
We're already working on upgrading esbuild to 0.15.13 for |
There's still value in using |
@pcattori that's a function expression/anonymous function though. You can't do this for example export (function hello() {
return "world"
}) satisfies () => number; |
Yea, if you want (1) The only way would be: export const hello = (function () {
return "world"
}) satisfies () => number; but the main benefit of named functions is that they can be referenced before their definition, which you would lose out on with this^. Personally, I prefer to use arrow functions, but yes its unfortunate for those who use |
21e8dbc
to
b035ae7
Compare
Btw, shameless plug 😅 https://tom-sherman.com/blog/remix-typescript-satisfies |
I guess you already knew that since this PR uses |
Looks like tests are failing because the Possible solution: eslint/eslint-plugin-markdown#114 (comment) |
LoaderFunction
type for usage with satisfies
in TypeScript 4.9satisfies
in combination with LoaderFunction
@pcattori Weird one, I wasn't actually aware of that type when I wrote the blog because I'm on an experimental channel (deffered) that doesn't have that type 😆 |
Is there any updated best practice/solution to solve this? I want to be able to take advantage of |
@moishinetzer The simplest way to do that would be to add a return type annotation to the loader eg. export async function loader({ context }: LoaderArgs): Promise<TypedResponse<{ posts: Post[], tags: string[] }>> {
const blog = new BlogData(context);
const [posts, tags] = await Promise.all([
blog.listAllPosts(),
blog.listAllTags(),
]);
return json({
posts,
tags,
});
}
const { posts, tags } = useLoaderData<typeof loader>(); You can use |
Co-authored-by: Michaël De Boey <info@michaeldeboey.be>
Co-authored-by: Michaël De Boey <info@michaeldeboey.be>
d3fd3ab
to
931ba33
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tom-sherman Could you rebase this to target main
?
Documentation & templates updates go directly to main
@@ -195,6 +195,34 @@ export default function Posts() { | |||
|
|||
Hey, that's pretty cool. We get a pretty solid degree of type safety even over a network request because it's all defined in the same file. Unless the network blows up while Remix fetches the data, you've got type safety in this component and its API (remember, the component is already its own API route). | |||
|
|||
We can take the type safety a step further in TypeScript 4.9.0 using the `satisfies` keyword. This will ensure that we always return a type of `Response` from the loader instead of accidentally returning another type which would result in a runtime error. | |||
|
|||
```ts filename=app/routes/posts/index.tsx lines=[1,23] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
```ts filename=app/routes/posts/index.tsx lines=[1,23] | |
```ts filename=app/routes/posts/index.tsx |
import type { LoaderFunction } from "@remix-run/node"; | ||
import { json } from "@remix-run/node"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import type { LoaderFunction } from "@remix-run/node"; | |
import { json } from "@remix-run/node"; | |
import type { LoaderFunction } from "@remix-run/node"; // or cloudflare/deno | |
import { json } from "@remix-run/node"; // or cloudflare/deno |
Looks like the upstream issues with eslint plugins and TS 4.9 have been resolved as That said, we are no longer using this tutorial in the v2 docs, so closing this PR. |
Closes: #
Testing Strategy:
I don't think this is needed as it's just a utility type. The interesting part happens in userland when we use the
satisfies
keyword.Right now because of the way you need to add types for loaders you can get into a situation where the return type of
useLoaderData
isnever
:ideally we'd like typescript to error when we return the string instead of a Response. We can use the new
satisfies
keyword for this:The loader now has a type error but it still allows for the inference of the return type to work with
useLoaderData
.