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

Allow useGlobalAction callback to take no arguments #648

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

MillanWangGadget
Copy link
Contributor

@MillanWangGadget MillanWangGadget commented Sep 19, 2024

  • UPDATE
    • Some Gadget template apps have type errors because they have useGlobalAction and no parameter in the callback.
      • Expected 1-2 arguments, but got 0
    • This actually works and should be allowed by the type

This will no longer have a type error with the default code snip
CleanShot 2024-09-19 at 16 26 40@2x

Copy link
Contributor

@danroberts danroberts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that we should be covering this case in react/src/utils when we do:

 * The return value of a `useAction`, `useGlobalAction`, `useBulkAction` etc hook.
 * Includes the data result object and a function for running the mutation.
 **/
export type ActionHookResult<Data = any, Variables extends AnyVariables = AnyVariables> = RequiredKeysOf<Variables> extends never
  ? [
      ActionHookState<Data, Variables>,
      (variables?: Variables, context?: Partial<OperationContext>) => Promise<ActionHookState<Data, Variables>>
    ]
  : [
      ActionHookState<Data, Variables>,
      (variables: Variables, context?: Partial<OperationContext>) => Promise<ActionHookState<Data, Variables>>
    ];
    ```
    
    maybe there is something wrong with that type?

@MillanWangGadget MillanWangGadget force-pushed the mill/allowUseGlobalActionToTakeNoParams branch from ef2e696 to 246f9cd Compare September 24, 2024 20:36
@MillanWangGadget
Copy link
Contributor Author

It seems to me that we should be covering this case in react/src/utils when we do:
maybe there is something wrong with that type?

Good call. It turns out that the Record<string,never> that global actions pass into ActionHookResultwas causing the callback type to have a mandatory variables argument parameter. Since globalActions cannot have required params, I've updated the return type to always have an optional variables argument

Comment on lines +130 to +132
? ActionHookResultWithOptionalCallbackVariables<Data, Variables>
: ActionHookResultWithRequiredCallbackVariables<Data, Variables>;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I tried doing a ternary chain here, but this causes a lot of type errors

@MillanWangGadget MillanWangGadget merged commit b30a841 into main Sep 26, 2024
9 checks passed
@MillanWangGadget MillanWangGadget deleted the mill/allowUseGlobalActionToTakeNoParams branch September 26, 2024 15:33
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