relay-tools / relay-hooks

Use Relay as React hooks
https://relay-tools.github.io/relay-hooks/docs/relay-hooks.html
MIT License
540 stars 56 forks source link

typesafe onMutation promise #132

Closed quant-daddy closed 3 years ago

quant-daddy commented 3 years ago

onMutation hook resolves with the mutation response or reject with any error (except when an onError callback is specified, in which case it will resolve with no value on errors).

Wouldn't it be a good idea to make the resolve value optional regardless of whether onError is specified or nor? Currently, if I specify onError, I still get the typescript type of resolve value as non-null and this could lead to runtime error.

morrys commented 3 years ago

Hi @skk2142, I would like to add this issue to the new release (4.0.0).

The solution that could be adopted is to change this: https://github.com/relay-tools/react-relay-mutation/blob/master/src/index.tsx#L90-L95

if (mergedConfig.onError) {
  mergedConfig.onError(error);
  resolve();
} else {
  reject(error);
}

in

if (mergedConfig.onError) {
  mergedConfig.onError(error);
  resolve(null);
} else {
  reject(error);
}

or

if (mergedConfig.onError) {
  mergedConfig.onError(error);
} 
reject(error);

and remove support for HOC Mutation.

morrys commented 3 years ago

Issue resolved with relay-hooks version 4.0.0