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-blog-tutorial): fix useActionData/useLoaderData usage #83

Conversation

MichaelDeBoey
Copy link
Member

@MichaelDeBoey MichaelDeBoey commented Nov 23, 2022

@MichaelDeBoey MichaelDeBoey force-pushed the fix-useActionData-useLoaderData-usage-blog-tutorial branch from 7604b78 to 6d7b9d8 Compare November 23, 2022 20:10
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.

Same comment as the other tutorial. Thanks!

@MichaelDeBoey MichaelDeBoey force-pushed the fix-useActionData-useLoaderData-usage-blog-tutorial branch 2 times, most recently from b5e5863 to ae1f7e9 Compare November 27, 2022 16:18
@mcansh
Copy link
Contributor

mcansh commented Nov 28, 2022

unfortunately this causes a bunch of typescript errors due to only returning the error for the specific input resulting in

Property 'email' does not exist on type 'SerializeObject<UndefinedToOptional<{ email: string; }>> | SerializeObject<UndefinedToOptional<{ password: string; }>>'.

this should be fixable in remix itself by concatenating all the keys into a single SerializeObject<UndefinedToOptional<{ password?: string; email?: string }>> type, but i'm not sure if others have feelings about not doing that

edit: we should probably run tsc on PRs like we do eslint

@MichaelDeBoey MichaelDeBoey force-pushed the fix-useActionData-useLoaderData-usage-blog-tutorial branch from ae1f7e9 to fc5d2de Compare November 28, 2022 18:20
@MichaelDeBoey
Copy link
Member Author

unfortunately this causes a bunch of typescript errors due to only returning the error for the specific input

@mcansh Fixed these errors

we should probably run tsc on PRs like we do eslint

Although that would be a good idea to not have these issues, it will require an npm install (& sometimes even more, like npx prisma migrate dev), otherwise TypeScript will complain 😢

@mcansh
Copy link
Contributor

mcansh commented Nov 28, 2022

it will require an npm install (& sometimes even more, like npx prisma migrate dev), otherwise TypeScript will complain 😢

oof yeah that's true 😞

@mcansh
Copy link
Contributor

mcansh commented Nov 28, 2022

unrelated to changes here, but looks like React now wants startTransition to return void or undefined

app/entry.client.tsx:7:5 - error TS2322: Type 'Root' is not assignable to type 'VoidOrUndefinedOnly'.

  7     hydrateRoot(
        ~~~~~~~~~~~~
  8       document,
    ~~~~~~~~~~~~~~~
... 
 11       </StrictMode>
    ~~~~~~~~~~~~~~~~~~~
 12     )
    ~~~~~

  node_modules/@types/react/index.d.ts:1107:38
    1107     export type TransitionFunction = () => VoidOrUndefinedOnly;
                                              ~~~~~~~~~~~~~~~~~~~~~~~~~
    The expected type comes from the return type of this signature.


Found 1 error.

error Command failed with exit code 1.
-  startTransition(() =>
+  startTransition(() => {
    hydrateRoot(
      document,
      <StrictMode>
        <RemixBrowser />
      </StrictMode>
    )
-  );
+  });

@MichaelDeBoey
Copy link
Member Author

unrelated to changes here, but looks like React now wants startTransition to return void or undefined

I'll create a separate PR for it 👍

@mcansh
Copy link
Contributor

mcansh commented Nov 28, 2022

I'll create a separate PR for it 👍

😬 #91 - you can take on the templates if you'd like 😉

@MichaelDeBoey
Copy link
Member Author

MichaelDeBoey commented Nov 28, 2022

oof yeah that's true 😞

Would love to find a solution for this though 🤔

Maybe a setup script for all extra stuff that needs to happen in order to make TS happy? 🤔

It would still take a long time to run npm install && npm run -s setup on all the example folders though 🤔😕

@mcansh
Copy link
Contributor

mcansh commented Nov 28, 2022

unless we can tell which example changed and only run tsc on that

@MichaelDeBoey
Copy link
Member Author

@mcansh Created an issue to discuss it further: #92

@MichaelDeBoey MichaelDeBoey merged commit 327deac into remix-run:main Nov 28, 2022
@MichaelDeBoey MichaelDeBoey deleted the fix-useActionData-useLoaderData-usage-blog-tutorial branch November 28, 2022 19:54
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.

Typecheck fails on blog-tutorial example
3 participants