hooks: useUpdate onError signature typescript issue

[soullivaneuh]

Since the v4 upgrade of react-admin, I have the following typing issue on the useUpdate function when I try to access the error.message property of the parameter provided by the onError function:

TS2571: Object is of type 'unknown'.
    23 |     {
    24 |       onError: (error) => {
  > 25 |         notify(`Error while updating: ${error.message}`);
       |                                         ^^^^^
    26 |       }
    27 |     }
    28 |   )

I also try to specify the type of the error like that: (error: Error) => ..., but this leads to the following another error:

TS2322: Type '(error: Error) => void' is not assignable to type '(error: unknown, variables: Partial<UseUpdateMutateParams<RaRecord>>, context: unknown) => void | Promise<unknown>'.
  Types of parameters 'error' and 'error' are incompatible.
    Type 'unknown' is not assignable to type 'Error'.
    22 |     },
    23 |     {
  > 24 |       onError: (error: Error) => {
       |       ^^^^^^^
    25 |         notify(`Error while updating: ${error.message}`);
    26 |       }
    27 |     }

Steps to reproduce:

1. Clone the referenced PR below
2. Run npm start

Related code:

Sandbox PR: soullivaneuh/react-admin-sandbox#1

Other information:

Environment

• React-admin version: v4.0.3
• Last version that did not exhibit the issue (if applicable): v3
• React version: v18

[slax57]

Hi!
Thanks for submitting this.
Indeed useUpdate and most of our react-query based hooks declare the error type as unknown.
This is probably something we could improve, but I need to discuss it with the team to say how and when.
I'm labeling it as enhancement for now.

In the meantime, here is some code inspired by the way we handle the default onError behaviour in the useEditController that you cas use as a workaround:

    const notify = useNotify();
    const [update] = useUpdate(
      'users',
      {
        id: 42,
        data: { email: 'foo@bar.com' },
      },
      {
        onError: (error: Error | string) => {
          notify(`Error while updating: ${typeof error === 'string'
          ? error
          : error.message}`);
        }
      }
    )

[fzaninotto]

react-query's useMutation is generic and accepts an Error type. We should support the same to let developers specify the error type on react-query based hooks.

const notify = useNotify();
    const [update] = useUpdate<User, Error | string>(
      'users',
      {
        id: 42,
        data: { email: 'foo@bar.com' },
      },
      {
        onError: (error) => { // <= react-query will type the error correctly
          notify(`Error while updating: ${typeof error === 'string'
          ? error
          : error.message}`);
        }
      }
    )

Until we implement that, you can narrow the type, as explained in React Query and TypeScript:

    const notify = useNotify();
    const [update] = useUpdate(
      'users',
      {
        id: 42,
        data: { email: 'foo@bar.com' },
      },
      {
        onError: (error) => {
          const message = error instanceOf Error ? error.message : error instanceOf String ? error : undefined;
          notify(`Error while updating: ${message}`);
        }
      }
    )

[soullivaneuh]

@fzaninotto I copy/paste your sample for trying, however it trigger me a lot of syntax error:

src/resources/credit-cards/components/CreditCardValidField.tsx:28:64 - error TS1005: ',' expected.

28         const message = error instanceOf Error ? error.message : error instanceOf String ? error : undefined;
                                                                  ~

src/resources/credit-cards/components/CreditCardValidField.tsx:28:72 - error TS1005: ',' expected.

28         const message = error instanceOf Error ? error.message : error instanceOf String ? error : undefined;
                                                                          ~~~~~~~~~~

src/resources/credit-cards/components/CreditCardValidField.tsx:28:83 - error TS1005: ',' expected.

28         const message = error instanceOf Error ? error.message : error instanceOf String ? error : undefined;
                                                                                     ~~~~~~

src/resources/credit-cards/components/CreditCardValidField.tsx:28:92 - error TS1005: ',' expected.

28         const message = error instanceOf Error ? error.message : error instanceOf String ? error : undefined;
                                                                                              ~~~~~

src/resources/credit-cards/components/CreditCardValidField.tsx:28:109 - error TS1005: ',' expected.

28         const message = error instanceOf Error ? error.message : error instanceOf String ? error : undefined;
                                                                                                               ~

src/resources/credit-cards/components/CreditCardValidField.tsx:30:16 - error TS1003: Identifier expected.

30         notify(`Error while updating: ${message}`);
                  ~~~~~~~~~~~~~~~~~~~~~~~~~

Am I missing something here? 🤔

[soullivaneuh]

For now, my workaround was to silent the error until resolution:

// @ts-expect-error https://github.com/marmelab/react-admin/issues/7653
onError: (error: Error) => {
  notify(`Echec de la mise à jour : ${error.message}`, {
    type: 'error',
  });
},

[slax57]

@soullivaneuh just to be sure, did you try with instanceOf or instanceof? (the latter being the correct syntax)

[soullivaneuh]

@slax57 Well... 🙃

Next time I'll re-read my copy/paste, thanks! 😉

My setup:

const [setup, { isLoading }] = useUpdate(
  'users',
  {
  },
  {
    onSuccess: () => {
      refresh();
    },
    onError: (error) => {
      const message = error instanceof Error ? error.message : error instanceof String ? error : undefined;
      notify(`Echec : ${message}`, {
        type: 'error',
      });
      refresh();
    },
  },
);

Works! However, this double ternary shows indeed the need of a stronger typing to be provided by useUpdate.

[soullivaneuh]

@slax57 I found a way to resolve the issue: #7698

Would you mind take a look? :-)