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(_official-jokes): fix useActionData/useLoaderData usage #84

Conversation

MichaelDeBoey
Copy link
Member

@MichaelDeBoey MichaelDeBoey commented Nov 23, 2022

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.

Thanks!

};

export const loader: LoaderFunction = async ({ request }) => {
export const loader = async ({ request }: LoaderArgs) => {
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest switching to DataFunctionArgs which can be used for both loaders and actions. Also, I'd personally switch this to a function declaration.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd suggest switching to DataFunctionArgs which can be used for both loaders and actions.

I rather prefer to use action/loader specific types tbh as that makes it more explicit imo & it's also more in line with ActionFunction/LoaderFunction
But that's a personal taste

I'd personally switch this to a function declaration

I opted to keep actions/loaders the way they are, to make the diff as small as possible.
Also: as you can see in the discussions of remix-run/remix#4675, satifies only works with arrow function

Copy link
Member

Choose a reason for hiding this comment

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

If satisfies only works for arrow functions then I'll probably not use it for this myself 🙁

In any case, I'm not really a maintainer anymore so you all can feel free to do whatever you'd like.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's wait and see what @mcansh & the rest of the team decide

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest switching to DataFunctionArgs which can be used for both loaders and actions. Also, I'd personally switch this to a function declaration.

that's kind of where i'm at currently in random side project apps

If satisfies only works for arrow functions then I'll probably not use it for this myself 🙁

it's only for variables/arrow functions unfortunately 😢, but we're also still on esbuild 0.15.11 due to some stdout issues in tests, so we don't support satisfies yet anyways

@MichaelDeBoey MichaelDeBoey force-pushed the fix-useActionData-useLoaderData-usage-jokes branch 4 times, most recently from bce442e to d56f8f2 Compare November 28, 2022 19:57
@MichaelDeBoey MichaelDeBoey requested a review from mcansh November 28, 2022 19:58
@MichaelDeBoey MichaelDeBoey force-pushed the fix-useActionData-useLoaderData-usage-jokes branch 4 times, most recently from 3acf602 to 6104b6e Compare November 28, 2022 21:43
@MichaelDeBoey MichaelDeBoey merged commit 7eeb2a5 into remix-run:main Nov 28, 2022
@MichaelDeBoey MichaelDeBoey deleted the fix-useActionData-useLoaderData-usage-jokes branch November 28, 2022 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants