Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

docs(decisions-0003): infer types for useLoaderData from loader type using generic #3704

Merged
merged 5 commits into from
Jul 15, 2022

Conversation

pcattori
Copy link
Contributor

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Jul 11, 2022

⚠️ No Changeset found

Latest commit: 0bb5f4c

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.

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

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fantastic! Thanks for taking the time to write it up. A couple of things:

  1. Let's talk about actions
  2. Let's talk about deferred (Jacob is working on implementing support for that)
  3. I forgot what this one was, but I knew I had three 😅 I'll think of it later.

@pcattori pcattori force-pushed the pedro/decision-infer-useloaderdata-from-loader branch from 0a58f74 to 0bf2d90 Compare July 11, 2022 20:24
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid 👍 This is good to merge with my suggestions IMO.


export default function Route() {
let dataGeneric = useLoaderData<MyLoaderData>() // <-- will be deprecated
let dataCast = useLoaderData() as MyLoaderData // <- use this instead
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work with our v2 plans, so let's go ahead and recommend what would be needed in that eventuality?

Suggested change
let dataCast = useLoaderData() as MyLoaderData // <- use this instead
let dataCast = useLoaderData() as any as MyLoaderData // <- use this instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our v2 plans are to return unknown in this case. You can type cast from unknown to any type without TS getting mad. So useLoaderData() as MyLoaderData will work for v2.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh? That's great!

@pcattori pcattori requested a review from kentcdodds July 15, 2022 02:30
@pcattori pcattori merged commit 7942655 into main Jul 15, 2022
@pcattori pcattori deleted the pedro/decision-infer-useloaderdata-from-loader branch July 15, 2022 02:31
dvargas92495 pushed a commit to dvargas92495/remix that referenced this pull request Jul 22, 2022
…ype using generic (remix-run#3704)

* docs(decisions-0003): infer types for `useLoaderData` from `loader` type using generic

* docs(decisions-0003): rewrite decision doc

include summary of current approach from v1.6.4

* docs(decisions-0003): rename file to match title

* docs(decisions-0003): fix typo

* docs(decisions-0003): pr feedback
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants