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: fix useActionData/useLoaderData usage #82

Conversation

MichaelDeBoey
Copy link
Member

@MichaelDeBoey MichaelDeBoey commented Nov 23, 2022

@MichaelDeBoey MichaelDeBoey force-pushed the fix-useActionData-useLoaderData-usage branch 3 times, most recently from df68b11 to 968bdcf Compare November 24, 2022 21:06
@MichaelDeBoey MichaelDeBoey force-pushed the fix-useActionData-useLoaderData-usage branch 2 times, most recently from 7ee65c2 to abe0d5b Compare November 27, 2022 16:18
@machour
Copy link
Collaborator

machour commented Nov 27, 2022

@MichaelDeBoey this is a big PR and makes it a little hard to validate.. I see you didn't use LoaderArgs in the few changes I looked at, should you?

@MichaelDeBoey
Copy link
Member Author

this is a big PR and makes it a little hard to validate

@machour I discussed it with @mcansh to have one big PR for all community examples & have separate PRs for each official example.

I see you didn't use LoaderArgs in the few changes I looked at, should you?

I'm using ActionArgs/LoaderArgs whenever they're actually used.
No need to add the type if it's not used.
This sometimes results in a removal of the type import.

@machour
Copy link
Collaborator

machour commented Nov 27, 2022

I'm using ActionArgs/LoaderArgs whenever they're actually used.

Aren't they needed for a proper inference?

@MichaelDeBoey
Copy link
Member Author

MichaelDeBoey commented Nov 27, 2022

Aren't they needed for a proper inference?

@machour No they aren't
Problem with previous usage & inference was the ActionFunction/LoaderFunction types
Now we have a normal function, where TS will infer the return type

@MichaelDeBoey MichaelDeBoey force-pushed the fix-useActionData-useLoaderData-usage branch from abe0d5b to 09f4b07 Compare November 28, 2022 19:57
@MichaelDeBoey MichaelDeBoey requested a review from mcansh November 28, 2022 21:53
@MichaelDeBoey MichaelDeBoey deleted the fix-useActionData-useLoaderData-usage branch December 6, 2022 15:16
@MichaelDeBoey MichaelDeBoey restored the fix-useActionData-useLoaderData-usage branch December 7, 2022 17:34
@MichaelDeBoey MichaelDeBoey reopened this Dec 7, 2022
@MichaelDeBoey MichaelDeBoey force-pushed the fix-useActionData-useLoaderData-usage branch from 09f4b07 to c74d3b4 Compare December 7, 2022 17:34
@mcansh
Copy link
Contributor

mcansh commented Dec 7, 2022

got through 90 of the easier to visually check ones, running a lil bash script to install, build, and type check the rest 😅 - almost there!

@MichaelDeBoey
Copy link
Member Author

MichaelDeBoey commented Dec 7, 2022

@mcansh Would be nice if we could add that script to a workflow to check on PRs (as explained in #92)

@mcansh
Copy link
Contributor

mcansh commented Dec 7, 2022

@mcansh Would be nice if we could add that script to a workflow to check on PRs (as explained in #92)

def, for now it's doing all of them and not only the changed ones https://gist.github.com/mcansh/ec38261fd0356ba050b9c10f1ed78c61

@MichaelDeBoey MichaelDeBoey merged commit 15d4066 into remix-run:main Dec 7, 2022
@MichaelDeBoey MichaelDeBoey deleted the fix-useActionData-useLoaderData-usage branch December 7, 2022 18:45
mcansh referenced this pull request in MichaelDeBoey/remix-examples Dec 16, 2022
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