Skip to content

Commit

Permalink
Allow no variables to be passed at all to useActions which don't requ…
Browse files Browse the repository at this point in the history
…ire variables

Antoine and I both created param-less actions recently, and went to run them using `mutate()` as returned by the react hook. Before this change, that fails with an internal error inside the hook, where we assumed that the variables were present. If you pass `mutate({})`, things work fine, but that seems superfluous. The types should have caught this if we were working in TS, but since we were in the Gadget editor, the type check is off.

So, this changes the types and the runtime code to allow passing nothing at all to the mutate function returned by the hook if there are no required variables. We're still just the client here, so the server will still validate that any really required params are present and return an error if they aren't passed.
  • Loading branch information
airhorns committed Aug 16, 2023
1 parent 8121350 commit e44360d
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 11 deletions.
56 changes: 55 additions & 1 deletion packages/react/spec/useAction.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { MockClientWrapper, createMockUrqlCient, mockUrqlClient } from "./testWr

describe("useAction", () => {
// these functions are typechecked but never run to avoid actually making API calls
const TestUseActionCanRunActionsWithVariables = () => {
const TestUseActionCanRunUpdateActionsWithVariables = () => {
const [_, mutate] = useAction(relatedProductsApi.user.update);

// can call with variables
Expand All @@ -32,6 +32,22 @@ describe("useAction", () => {
void mutate({ foo: "123" });
};

const TestUseActionCanRunCreateActionsWithVariables = () => {
const [_, mutate] = useAction(relatedProductsApi.user.create);

// can call with variables
void mutate({ user: { email: "foo@bar.com" } });

// can call with no model variables
void mutate({});

// can call with no variables at all
void mutate();

// @ts-expect-error can't call with variables that don't belong to the model
void mutate({ foo: "123" });
};

const TestUseActionCanRunWithoutModelApiIdentifier = () => {
const [_, mutate] = useAction(relatedProductsApi.unambiguous.update);

Expand Down Expand Up @@ -419,4 +435,42 @@ describe("useAction", () => {
`[Error: Invalid arguments found in variables. Did you mean to use ({ ambiguous: { ... } })?]`
);
});

test("can run a mutation which takes no variables without passing any", async () => {
const { result, rerender } = renderHook(() => useAction(relatedProductsApi.user.create), {
wrapper: MockClientWrapper(relatedProductsApi),
});

let mutationPromise: any;
act(() => {
mutationPromise = result.current[1]();
});

expect(mockUrqlClient.executeMutation).toBeCalledTimes(1);

mockUrqlClient.executeMutation.pushResponse("createUser", {
data: {
updateUser: {
success: true,
user: {
id: "123",
email: "test@test.com",
},
},
},
stale: false,
hasNext: false,
});

await act(async () => {
await mutationPromise;
});

const beforeObject = result.current[0]!;
expect(beforeObject).toBeTruthy();

rerender();

expect(result.current[0]).toBe(beforeObject);
});
});
5 changes: 3 additions & 2 deletions packages/react/src/useAction.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { ActionFunction, DefaultSelection, GadgetRecord, LimitToKnownKeys, Select } from "@gadgetinc/api-client-core";
import { actionOperation, capitalizeIdentifier, get, hydrateRecord } from "@gadgetinc/api-client-core";
import { useCallback, useContext, useMemo } from "react";
import type { AnyVariables, UseMutationState } from "urql";
import type { AnyVariables, OperationContext, UseMutationState } from "urql";
import { GadgetUrqlClientContext } from "./GadgetProvider.js";
import { useGadgetMutation } from "./useGadgetMutation.js";
import { useStructuralMemo } from "./useStructuralMemo.js";
Expand Down Expand Up @@ -75,7 +75,8 @@ export const useAction = <
return [
transformedResult,
useCallback(
async (variables, context) => {
async (variables: F["variablesType"], context?: Partial<OperationContext>) => {
variables ??= {};
if (action.hasAmbiguousIdentifier) {
if (Object.keys(variables).some((key) => !action.paramOnlyVariables?.includes(key) && key !== action.modelApiIdentifier)) {
throw Error(`Invalid arguments found in variables. Did you mean to use ({ ${action.modelApiIdentifier}: { ... } })?`);
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/useBulkAction.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { BulkActionFunction, DefaultSelection, GadgetRecord, LimitToKnownKeys, Select } from "@gadgetinc/api-client-core";
import { actionOperation, capitalizeIdentifier, get, hydrateRecordArray } from "@gadgetinc/api-client-core";
import { useCallback, useMemo } from "react";
import type { UseMutationState } from "urql";
import type { OperationContext, UseMutationState } from "urql";
import { useGadgetMutation } from "./useGadgetMutation.js";
import { useStructuralMemo } from "./useStructuralMemo.js";
import type { ActionHookResult, OptionsType } from "./utils.js";
Expand Down Expand Up @@ -71,7 +71,7 @@ export const useBulkAction = <
return [
transformedResult,
useCallback(
async (variables, context) => {
async (variables: F["variablesType"], context?: Partial<OperationContext>) => {
// Adding the model's additional typename ensures document cache will properly refresh, regardless of whether __typename was
// selected (and sometimes we can't even select it, like delete actions!)
const result = await runMutation(variables, {
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/useGlobalAction.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { GlobalActionFunction } from "@gadgetinc/api-client-core";
import { get, globalActionOperation } from "@gadgetinc/api-client-core";
import { useCallback, useMemo } from "react";
import type { UseMutationState } from "urql";
import type { OperationContext, UseMutationState } from "urql";
import { useGadgetMutation } from "./useGadgetMutation.js";
import type { ActionHookResult } from "./utils.js";
import { ErrorWrapper } from "./utils.js";
Expand Down Expand Up @@ -41,7 +41,7 @@ export const useGlobalAction = <F extends GlobalActionFunction<any>>(
return [
transformedResult,
useCallback(
async (variables, context) => {
async (variables: F["variablesType"], context?: Partial<OperationContext>) => {
const result = await runMutation(variables, context);
return processResult({ fetching: false, ...result }, action);
},
Expand Down
20 changes: 16 additions & 4 deletions packages/react/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,26 @@ export interface ActionHookState<Data = any, Variables extends AnyVariables = Re
operation?: Operation<Data, Variables>;
}

export type RequiredKeysOf<BaseType> = Exclude<
{
[Key in keyof BaseType]: BaseType extends Record<Key, BaseType[Key]> ? Key : never;
}[keyof BaseType],
undefined
>;

/**
* The return value of a `useAction`, `useGlobalAction`, `useBulkAction` etc hook.
* Includes the data result object and a function for running the mutation.
**/
export declare type ActionHookResult<Data = any, Variables extends AnyVariables = AnyVariables> = [
ActionHookState<Data, Variables>,
(variables: Variables, context?: Partial<OperationContext>) => Promise<ActionHookState<Data, Variables>>
];
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>>
];

export const noProviderErrorMessage = `Could not find a client in the context of Provider. Please ensure you wrap the root component in a <Provider>`;

Expand Down

0 comments on commit e44360d

Please sign in to comment.