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

Remix singleFetch and FieldError type #100

Open
luzat opened this issue Jun 29, 2024 · 8 comments
Open

Remix singleFetch and FieldError type #100

luzat opened this issue Jun 29, 2024 · 8 comments

Comments

@luzat
Copy link

luzat commented Jun 29, 2024

I have set up my Remix project to use unstable_singleFetch. This means that I am generally not using return json({ … }) (even though I sometimes could):

export const action = defineAction(async ({ request }) => {
  const { errors, data, receivedValues: defaultValues } = await getValidatedFormData<FormData>(request, resolver)

  if (errors) {
    return {
      errors, // <= error
      defaultValues,
    }
  }

  return {
    data,
  }
})

This gives a TypeScript error, because FieldError elements contains a ref?: Ref which cannot be serialized.

I think ref will always be undefined here, though, and getValidatedFormData's return type should probably reflect that. This may need to be solved for other non-hook methods, too. As a hack, I did this:

import type {
  BrowserNativeObject,
  DeepRequired,
  FieldError,
  FieldValues,
  GlobalError,
  IsAny,
  Merge,
} from 'react-hook-form'
import { getValidatedFormData, useRemixForm } from 'remix-hook-form'

// …

export type FieldErrorNoRef = FieldError & {
  root?: FieldErrorNoRef
  ref?: undefined
}

export type FieldErrorsNoRefImpl<T extends FieldValues = FieldValues> = {
  [K in keyof T]?: T[K] extends BrowserNativeObject | Blob
    ? FieldErrorNoRef
    : K extends 'root' | `root.${string}`
      ? GlobalError
      : T[K] extends object
        ? Merge<FieldErrorNoRef, FieldErrorsNoRefImpl<T[K]>>
        : FieldErrorNoRef
}

export type FieldErrorsNoRef<T extends FieldValues = FieldValues> = Partial<
  FieldValues extends IsAny<FieldValues> ? any : FieldErrorsNoRefImpl<DeepRequired<T>>
> & {
  root?: Record<string, GlobalError> & GlobalError
}

export const action = defineAction(async ({ request }) => {
  const { errors, data, receivedValues: defaultValues } = await getValidatedFormData<FormData>(request, resolver)

  if (errors) {
    return {
      errors: errors as FieldErrorsNoRef, // <= No error
      defaultValues,
    }
  }

  return {
    data,
  }
})

I am not sure if this solves every problem with singleFetch, but the cast avoids TypeScript errors. I will probably wrap all relevant methods for now, but it might suffice to define FieldErrorsNoRef (and the other relevant types) as above and change the cast in validateFormData:

return { errors: errors as FieldErrorsNoRef<T>, data: undefined };

This is obviously not very elegant as it mirrors the types from react-hook-form.

@timvandam
Copy link

running into this as well!

@AlemTuzlak
Copy link
Contributor

I'll try to fix this asap, hopefully very soon

@AlemTuzlak
Copy link
Contributor

@luzat what version of Remix is this on? Is it prior to 2.11.x? I don't get the same errors in the latest version, or I'm missing something?

@luzat
Copy link
Author

luzat commented Aug 27, 2024

@AlemTuzlak This was with Remix 2.10.0 I think (current package versions from bug report date), but I can still reproduce it with Remix 2.11.2 and current versions of remix-hook-form and react-hook-form. Here's a more minimal test:

import { zodResolver } from '@hookform/resolvers/zod'
import { unstable_defineAction as defineAction } from '@remix-run/node'
import { getValidatedFormData } from 'remix-hook-form'
import { z } from 'zod'

const schema = z.object({
  value: z.string(),
})

type FormData = Zod.infer<typeof schema>

const resolver = zodResolver(schema)

export const action = defineAction(async ({ request }) => {
  const { errors } = await getValidatedFormData<FormData>(request, resolver)
  return errors ? { errors } : {}
})

TypeScript error:

app/routes/test.tsx:14:36 - error TS2345: Argument of type '({ request }: ActionFunctionArgs) => Promise<{ errors: FieldErrors<{ value: string; }>; } | { errors?: undefined; }>' is not assignable to parameter of type 'Action'.
  Type 'Promise<{ errors: FieldErrors<{ value: string; }>; } | { errors?: undefined; }>' is not assignable to type 'MaybePromise<DataFunctionReturnValue>'.
    Type 'Promise<{ errors: FieldErrors<{ value: string; }>; } | { errors?: undefined; }>' is not assignable to type '{ [key: string]: Serializable; [key: number]: Serializable; [key: symbol]: Serializable; } | Map<Serializable, Serializable> | Set<...> | Promise<...> | Promise<...>'.
      Type 'Promise<{ errors: FieldErrors<{ value: string; }>; } | { errors?: undefined; }>' is not assignable to type 'Promise<Serializable>'.
        Type '{ errors: FieldErrors<{ value: string; }>; } | { errors?: undefined; }' is not assignable to type 'Serializable'.
          Type '{ errors: FieldErrors<{ value: string; }>; }' is not assignable to type 'Serializable'.
            Type '{ errors: FieldErrors<{ value: string; }>; }' is not assignable to type '{ [key: string]: Serializable; [key: number]: Serializable; [key: symbol]: Serializable; }'.
              Property 'errors' is incompatible with index signature.
                Type 'FieldErrors<{ value: string; }>' is not assignable to type 'Serializable'.
                  Type 'FieldErrors<{ value: string; }>' is not assignable to type '{ [key: string]: Serializable; [key: number]: Serializable; [key: symbol]: Serializable; }'.
                    Property 'value' is incompatible with index signature.
                      Type 'FieldError' is not assignable to type 'Serializable'.

14 export const action = defineAction(async ({ request }) => {
                                      ~~~~~~~~~~~~~~~~~~~~~~~~

With FieldError not being serializable because its ref could refer to some element according to its type. tsconfig.json contains the option:

{
  "compilerOptions": {
    "types": ["@remix-run/node", "vite/client", "@remix-run/react/future/single-fetch.d.ts"],
    …
  }
  …
}

Package versions are 2.11.2 for Remix, 5.0.4 for remix-hook-form, 7.53.0 for react-hook-form and 5.5.4 for TypeScript.

This is to be expected, given the FieldError type, which is returned by resolvers. One can remove defineAction to skip the type check completely. This is probably not too bad, because resolvers should in most cases return a type which is serializable by turbo-stream and useRemixForm doesn't care about the return type of the action method, but it'd be certainly cleaner to get a serializable type. Don't have any idea how to get that asserted by TypeScript more easily than with the redefined types and a cast I provided above, though.

@AlemTuzlak
Copy link
Contributor

@luzat ahh okay the defineAction was the thing that was missing for me, FYI this API is completely removed in Remix on this PR:
remix-run/remix#9893 and they fixed some type-safety which might make this problem completely go away in the future release. I'll see if there is an easier way to exclude the ref without copy/pasting the internal types from react-hook-form in the meantime.

@luzat
Copy link
Author

luzat commented Aug 27, 2024

@AlemTuzlak Looking at this, it's certainly not that important to fix the types currently. If the types are to be improved in the future, it might make more sense to first include an option or helper in react-hook-form, to (optionally) return Ref-free types.

@AlemTuzlak
Copy link
Contributor

I'm just a bit hesitant to add a fix for this now as there is a lot of API churn around single_fetch and there is no guarantee that this will be the case. I definitely plan to do something about it if it doesn't go away, until then your workaround will work for anyone on the unstable feature.

@AlemTuzlak
Copy link
Contributor

@luzat sorry to do this again, but I saw the latest release of remix fixed some type issues with single fetch, could you check if this is still an issue?

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

No branches or pull requests

3 participants