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

fix(docs): switch to using generic type for useLoaderData #4456

Conversation

tomcorey26
Copy link

@tomcorey26 tomcorey26 commented Oct 31, 2022

The blog tutorial has you set the typing for useLoaderData by using the type assertion as LoaderData. But typescript shows an error because of the way the hook is typed:

useLoaderData<T = AppData>(): SerializeFrom<T> 

Since the Post model we created in this tutorial contains two fields that are Date (createdAt and updatedAt), typescript complains because it expects the serialized type where they are both string

Conversion of type 'SerializeObject<UndefinedToOptional<LoaderData>>' to type 'LoaderData' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  The types of 'post.createdAt' are incompatible between these types.
    Type 'string' is not comparable to type 'Date'.ts(2352)

So I updated the docs to pass in the type as a generic instead useLoaderData<LoaderData>().

Also an alternative way to fix the error is to just do the assertion with the SerializeFrom util. e.g

import type { LoaderFunction, SerializeFrom } from "@remix-run/node";

const { post, html } = useLoaderData() as SerializeFrom<LoaderData>;

Not sure if this way would be preferred

@changeset-bot
Copy link

changeset-bot bot commented Oct 31, 2022

⚠️ No Changeset found

Latest commit: 44bdaf4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Oct 31, 2022

Hi @tomcorey26,

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 CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected].

Thanks!

- The Remix team

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Oct 31, 2022

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@pcattori
Copy link
Contributor

Superceded by #4684

@pcattori pcattori closed this Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants