From b84da408e2e3ae76088ef14b44cf262c9fed7685 Mon Sep 17 00:00:00 2001 From: Vyacheslav Matyukhin Date: Fri, 29 Sep 2023 15:08:14 -0600 Subject: [PATCH 1/4] useMutationForm hook --- .../[slug]/EditSquiggleSnippetModel.tsx | 107 ++++++++---------- packages/hub/src/hooks/useMutationForm.ts | 44 +++++++ 2 files changed, 93 insertions(+), 58 deletions(-) create mode 100644 packages/hub/src/hooks/useMutationForm.ts diff --git a/packages/hub/src/app/models/[owner]/[slug]/EditSquiggleSnippetModel.tsx b/packages/hub/src/app/models/[owner]/[slug]/EditSquiggleSnippetModel.tsx index 8274a68bc9..7f199fe494 100644 --- a/packages/hub/src/app/models/[owner]/[slug]/EditSquiggleSnippetModel.tsx +++ b/packages/hub/src/app/models/[owner]/[slug]/EditSquiggleSnippetModel.tsx @@ -1,9 +1,15 @@ import { FC, useMemo, useState } from "react"; -import { FormProvider, useFieldArray, useForm } from "react-hook-form"; +import { FormProvider, useFieldArray } from "react-hook-form"; import { graphql, useFragment } from "react-relay"; import { PlaygroundToolbarItem } from "@quri/squiggle-components"; import { Button, LinkIcon, TextTooltip, useToast } from "@quri/ui"; +import { + SquigglePlaygroundVersionPicker, + SquiggleVersionShower, + VersionedSquigglePlayground, + type SquiggleVersion, +} from "@quri/versioned-playground"; import { EditSquiggleSnippetModel$key } from "@/__generated__/EditSquiggleSnippetModel.graphql"; import { @@ -11,33 +17,9 @@ import { RelativeValuesExportInput, } from "@/__generated__/EditSquiggleSnippetModelMutation.graphql"; import { EditModelExports } from "@/components/exports/EditModelExports"; -import { useAsyncMutation } from "@/hooks/useAsyncMutation"; import { useAvailableHeight } from "@/hooks/useAvailableHeight"; +import { useMutationForm } from "@/hooks/useMutationForm"; import { extractFromGraphqlErrorUnion } from "@/lib/graphqlHelpers"; -import { - SquigglePlaygroundVersionPicker, - VersionedSquigglePlayground, - type SquiggleVersion, - SquiggleVersionShower, -} from "@quri/versioned-playground"; - -export const Mutation = graphql` - mutation EditSquiggleSnippetModelMutation( - $input: MutationUpdateSquiggleSnippetModelInput! - ) { - result: updateSquiggleSnippetModel(input: $input) { - __typename - ... on BaseError { - message - } - ... on UpdateSquiggleSnippetResult { - model { - ...EditSquiggleSnippetModel - } - } - } - } -`; type FormShape = { code: string; @@ -51,8 +33,6 @@ type Props = { }; export const EditSquiggleSnippetModel: FC = ({ modelRef }) => { - const toast = useToast(); - const model = useFragment( graphql` fragment EditSquiggleSnippetModel on Model { @@ -96,7 +76,7 @@ export const EditSquiggleSnippetModel: FC = ({ modelRef }) => { "SquiggleSnippet" ); - const { height, ref } = useAvailableHeight(); + const toast = useToast(); const initialFormValues: FormShape = useMemo(() => { return { @@ -111,8 +91,42 @@ export const EditSquiggleSnippetModel: FC = ({ modelRef }) => { }; }, [content, revision.relativeValuesExports]); - const form = useForm({ + const { form, onSubmit, inFlight } = useMutationForm< + FormShape, + EditSquiggleSnippetModelMutation, + "UpdateSquiggleSnippetResult" + >({ defaultValues: initialFormValues, + mutation: graphql` + mutation EditSquiggleSnippetModelMutation( + $input: MutationUpdateSquiggleSnippetModelInput! + ) { + result: updateSquiggleSnippetModel(input: $input) { + __typename + ... on BaseError { + message + } + ... on UpdateSquiggleSnippetResult { + model { + ...EditSquiggleSnippetModel + } + } + } + } + `, + expectedTypename: "UpdateSquiggleSnippetResult", + formDataToVariables: (formData) => ({ + input: { + content: { + code: formData.code, + version, + }, + relativeValuesExports: formData.relativeValuesExports, + slug: model.slug, + owner: model.owner.slug, + }, + }), + onCompleted: () => toast("Saved", "confirmation"), }); // could version picker be part of the form? @@ -127,31 +141,6 @@ export const EditSquiggleSnippetModel: FC = ({ modelRef }) => { control: form.control, }); - const [saveMutation, saveInFlight] = useAsyncMutation< - EditSquiggleSnippetModelMutation, - "UpdateSquiggleSnippetResult" - >({ - mutation: Mutation, - expectedTypename: "UpdateSquiggleSnippetResult", - }); - - const save = form.handleSubmit((formData) => { - saveMutation({ - variables: { - input: { - content: { - code: formData.code, - version, - }, - relativeValuesExports: formData.relativeValuesExports, - slug: model.slug, - owner: model.owner.slug, - }, - }, - onCompleted: () => toast("Saved", "confirmation"), - }); - }); - const onCodeChange = (code: string) => { form.setValue("code", code); }; @@ -165,9 +154,11 @@ export const EditSquiggleSnippetModel: FC = ({ modelRef }) => { setDefaultCode(form.getValues("code")); }; + const { height, ref } = useAvailableHeight(); + return ( -
+
= ({ modelRef }) => { {model.isEditable && ( diff --git a/packages/hub/src/hooks/useMutationForm.ts b/packages/hub/src/hooks/useMutationForm.ts new file mode 100644 index 0000000000..70831b121a --- /dev/null +++ b/packages/hub/src/hooks/useMutationForm.ts @@ -0,0 +1,44 @@ +import { FieldValues, UseFormProps, useForm } from "react-hook-form"; +import { GraphQLTaggedNode, VariablesOf } from "relay-runtime"; + +import { CommonMutationParameters, useAsyncMutation } from "./useAsyncMutation"; + +export function useMutationForm< + FormShape extends FieldValues = never, + TMutation extends CommonMutationParameters = never, + TTypename extends string = never, +>({ + mutation, + expectedTypename, + formDataToVariables, + defaultValues, + onCompleted, +}: { + mutation: GraphQLTaggedNode; + expectedTypename: TTypename; + // This is unfortunately not strictly type-safe: if you return extra variables that are not needed for mutation, TypeScript won't complain. + // See also: https://stackoverflow.com/questions/72111571/typescript-exact-return-type-of-function + // This could be solved by converting the return type to generic, but I expect that the lack of partial type parameters in TypeScript + // would get in the way, so I won't even try. + formDataToVariables: (data: FormShape) => VariablesOf; + onCompleted?: ( + // OkResult, see also: useAsyncMutation + result: Extract + ) => void; +} & Pick, "defaultValues">) { + const form = useForm({ defaultValues }); + + const [runMutation, inFlight] = useAsyncMutation({ + mutation, + expectedTypename, + }); + + const onSubmit = form.handleSubmit((formData) => { + runMutation({ + variables: formDataToVariables(formData), + onCompleted, + }); + }); + + return { form, onSubmit, inFlight }; +} From 8f72b96c1870f6316c8f4a45f3434d56c57f9d3d Mon Sep 17 00:00:00 2001 From: Vyacheslav Matyukhin Date: Fri, 29 Sep 2023 15:54:16 -0600 Subject: [PATCH 2/4] useMutationForm hook --- .../[slug]/EditSquiggleSnippetModel.tsx | 6 +- packages/hub/src/app/new/group/NewGroup.tsx | 48 +++----- packages/hub/src/app/new/model/NewModel.tsx | 109 ++++++++---------- .../choose-username/ChooseUsername.tsx | 77 ++++++------- .../src/components/ui/MutationModalAction.tsx | 42 +++---- packages/hub/src/hooks/useMutationForm.ts | 37 ++++-- 6 files changed, 149 insertions(+), 170 deletions(-) diff --git a/packages/hub/src/app/models/[owner]/[slug]/EditSquiggleSnippetModel.tsx b/packages/hub/src/app/models/[owner]/[slug]/EditSquiggleSnippetModel.tsx index 7f199fe494..73046257a2 100644 --- a/packages/hub/src/app/models/[owner]/[slug]/EditSquiggleSnippetModel.tsx +++ b/packages/hub/src/app/models/[owner]/[slug]/EditSquiggleSnippetModel.tsx @@ -3,7 +3,7 @@ import { FormProvider, useFieldArray } from "react-hook-form"; import { graphql, useFragment } from "react-relay"; import { PlaygroundToolbarItem } from "@quri/squiggle-components"; -import { Button, LinkIcon, TextTooltip, useToast } from "@quri/ui"; +import { Button, LinkIcon, TextTooltip } from "@quri/ui"; import { SquigglePlaygroundVersionPicker, SquiggleVersionShower, @@ -76,8 +76,6 @@ export const EditSquiggleSnippetModel: FC = ({ modelRef }) => { "SquiggleSnippet" ); - const toast = useToast(); - const initialFormValues: FormShape = useMemo(() => { return { code: content.code, @@ -126,7 +124,7 @@ export const EditSquiggleSnippetModel: FC = ({ modelRef }) => { owner: model.owner.slug, }, }), - onCompleted: () => toast("Saved", "confirmation"), + confirmation: "Saved", }); // could version picker be part of the form? diff --git a/packages/hub/src/app/new/group/NewGroup.tsx b/packages/hub/src/app/new/group/NewGroup.tsx index 1d7f5f408f..5d4d50d6d1 100644 --- a/packages/hub/src/app/new/group/NewGroup.tsx +++ b/packages/hub/src/app/new/group/NewGroup.tsx @@ -2,15 +2,15 @@ import { useSession } from "next-auth/react"; import { useRouter } from "next/navigation"; import { FC } from "react"; -import { FormProvider, useForm } from "react-hook-form"; +import { FormProvider } from "react-hook-form"; import { graphql } from "relay-runtime"; -import { Button, useToast } from "@quri/ui"; +import { Button } from "@quri/ui"; import { NewGroupMutation } from "@/__generated__/NewGroupMutation.graphql"; import { H1 } from "@/components/ui/Headers"; import { SlugFormField } from "@/components/ui/SlugFormField"; -import { useAsyncMutation } from "@/hooks/useAsyncMutation"; +import { useMutationForm } from "@/hooks/useMutationForm"; import { groupRoute } from "@/routes"; const Mutation = graphql` @@ -33,44 +33,34 @@ const Mutation = graphql` export const NewGroup: FC = () => { useSession({ required: true }); - const toast = useToast(); + const router = useRouter(); type FormShape = { slug: string | undefined; }; - const form = useForm({ + const { form, onSubmit, inFlight } = useMutationForm< + FormShape, + NewGroupMutation, + "CreateGroupResult" + >({ defaultValues: {}, mode: "onChange", - }); - - const router = useRouter(); - - const [saveMutation, isSaveInFlight] = useAsyncMutation({ mutation: Mutation, expectedTypename: "CreateGroupResult", blockOnSuccess: true, - }); - - const save = form.handleSubmit(async (data) => { - const slug = data.slug; - if (!slug) { - // shouldn't happen but satisfies Typescript - toast("Slug is undefined", "error"); - return; - } - await saveMutation({ - variables: { - input: { slug }, - }, - onCompleted() { - router.push(groupRoute({ slug })); + formDataToVariables: (data) => ({ + input: { + slug: data.slug ?? "", // shouldn't happen, but satisfies TypeScript }, - }); + }), + onCompleted(result) { + router.push(groupRoute({ slug: result.group.slug })); + }, }); return ( - +

New Group

@@ -82,8 +72,8 @@ export const NewGroup: FC = () => { />
diff --git a/packages/hub/src/components/ui/MutationModalAction.tsx b/packages/hub/src/components/ui/MutationModalAction.tsx index ab682b6979..93afa5a430 100644 --- a/packages/hub/src/components/ui/MutationModalAction.tsx +++ b/packages/hub/src/components/ui/MutationModalAction.tsx @@ -1,19 +1,12 @@ -import { FC, PropsWithChildren, ReactNode, useEffect, useState } from "react"; -import { - DefaultValues, - FieldPath, - FieldValues, - useForm, -} from "react-hook-form"; +import { FC, PropsWithChildren, ReactNode } from "react"; +import { DefaultValues, FieldPath, FieldValues } from "react-hook-form"; import { GraphQLTaggedNode, VariablesOf } from "relay-runtime"; -import { DropdownMenuActionItem, EditIcon, IconProps } from "@quri/ui"; +import { IconProps } from "@quri/ui"; import { FormModal } from "@/components/ui/FormModal"; -import { - CommonMutationParameters, - useAsyncMutation, -} from "@/hooks/useAsyncMutation"; +import { CommonMutationParameters } from "@/hooks/useAsyncMutation"; +import { useMutationForm } from "@/hooks/useMutationForm"; import { DropdownMenuModalActionItem } from "@quri/ui"; type CommonProps< @@ -51,23 +44,20 @@ function MutationFormModal< }: PropsWithChildren> & { title: string; }): ReactNode { - const form = useForm({ + const { form, onSubmit, inFlight } = useMutationForm< + TFormShape, + TMutation, + TTypename + >({ mode: "onChange", defaultValues, - }); - const [runMutation, inFlight] = useAsyncMutation({ mutation, expectedTypename, - }); - - const save = form.handleSubmit((data) => { - runMutation({ - variables: formDataToVariables(data), - onCompleted(data) { - onCompleted?.(data); - close(); - }, - }); + formDataToVariables, + onCompleted(data) { + onCompleted?.(data); + close(); + }, }); return ( @@ -77,7 +67,7 @@ function MutationFormModal< submitText={submitText} form={form} initialFocus={initialFocus} - onSubmit={save} + onSubmit={onSubmit} inFlight={inFlight} > {children} diff --git a/packages/hub/src/hooks/useMutationForm.ts b/packages/hub/src/hooks/useMutationForm.ts index 70831b121a..345e073a8e 100644 --- a/packages/hub/src/hooks/useMutationForm.ts +++ b/packages/hub/src/hooks/useMutationForm.ts @@ -1,5 +1,5 @@ import { FieldValues, UseFormProps, useForm } from "react-hook-form"; -import { GraphQLTaggedNode, VariablesOf } from "relay-runtime"; +import { VariablesOf } from "relay-runtime"; import { CommonMutationParameters, useAsyncMutation } from "./useAsyncMutation"; @@ -8,29 +8,44 @@ export function useMutationForm< TMutation extends CommonMutationParameters = never, TTypename extends string = never, >({ + // useForm params + defaultValues, + mode, + // useAsyncMutation params mutation, expectedTypename, - formDataToVariables, - defaultValues, + blockOnSuccess, + confirmation, + // runMutation params onCompleted, + // bridge from form to runMutation + formDataToVariables, }: { - mutation: GraphQLTaggedNode; - expectedTypename: TTypename; // This is unfortunately not strictly type-safe: if you return extra variables that are not needed for mutation, TypeScript won't complain. // See also: https://stackoverflow.com/questions/72111571/typescript-exact-return-type-of-function // This could be solved by converting the return type to generic, but I expect that the lack of partial type parameters in TypeScript // would get in the way, so I won't even try. formDataToVariables: (data: FormShape) => VariablesOf; - onCompleted?: ( - // OkResult, see also: useAsyncMutation - result: Extract - ) => void; -} & Pick, "defaultValues">) { - const form = useForm({ defaultValues }); +} & Pick, "defaultValues" | "mode"> & + Pick< + Parameters>[0], + // I could just pass everything but I'm worried about potential name collisions, and it's easy to whitelist everything necessary. + // We have to list all these when we pass them to `useAsyncMutation`, anyway. + "mutation" | "expectedTypename" | "blockOnSuccess" | "confirmation" + > & + // TODO - with `useAsyncMutation`, submitted data is available because `onCompleted` is usually a closure over it. + // For `useMutationForm`, it'd be useful to pass it to `onCompleted` explicitly, `onCompleted(result, submittedData)`. + Pick< + Parameters>[0]>[0], + "onCompleted" + >) { + const form = useForm({ defaultValues, mode }); const [runMutation, inFlight] = useAsyncMutation({ mutation, expectedTypename, + blockOnSuccess, + confirmation, }); const onSubmit = form.handleSubmit((formData) => { From c890b23dbfd59ca78e779212823f4591a1f315fe Mon Sep 17 00:00:00 2001 From: Vyacheslav Matyukhin Date: Fri, 29 Sep 2023 16:02:38 -0600 Subject: [PATCH 3/4] comments --- packages/hub/src/hooks/useAsyncMutation.ts | 6 +++--- packages/hub/src/hooks/useMutationForm.ts | 10 ++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/hub/src/hooks/useAsyncMutation.ts b/packages/hub/src/hooks/useAsyncMutation.ts index 33a7b4a607..de31294da0 100644 --- a/packages/hub/src/hooks/useAsyncMutation.ts +++ b/packages/hub/src/hooks/useAsyncMutation.ts @@ -1,8 +1,8 @@ +import { useState } from "react"; import { UseMutationConfig, useMutation } from "react-relay"; +import { GraphQLTaggedNode, MutationParameters } from "relay-runtime"; import { useToast } from "@quri/ui"; -import { useState } from "react"; -import { GraphQLTaggedNode, MutationParameters } from "relay-runtime"; export type CommonMutationParameters = MutationParameters & { @@ -22,7 +22,7 @@ export type CommonMutationParameters = }; /** - * Like the basic `useMutation`, this function returns a `[runMutation, isMutationInFlight]` pair. + * Like the basic `useMutation` from Relay, this function returns a `[runMutation, isMutationInFlight]` pair. * But unlike `useMutation`, returned `runMutation` is async and has more convenient `onCompleted` callback (it receives only an expected fragment, unwrapped from the result union). * Also, all errors will be displayed as notifications automatically. */ diff --git a/packages/hub/src/hooks/useMutationForm.ts b/packages/hub/src/hooks/useMutationForm.ts index 345e073a8e..d357f4c2ff 100644 --- a/packages/hub/src/hooks/useMutationForm.ts +++ b/packages/hub/src/hooks/useMutationForm.ts @@ -3,6 +3,16 @@ import { VariablesOf } from "relay-runtime"; import { CommonMutationParameters, useAsyncMutation } from "./useAsyncMutation"; +/** + * This hook ties together `useForm` and `useAsyncMutation`, which is a very common pattern for forms that submit data to our backend through mutations. + * It forwards some of its parameters to `useForm`, and others to `useAsyncMutation`, and converts form's data to mutation's variables with `formDataToVariables` function. + * + * See also: + * - `` if your form is available through a Dropdown menu + * - `useAsyncMutation` if your mutation doesn't require a form, and for the details on parameters for this function + * + * All generic type parameters to this function default to `never`, so you'll have to set them explicitly to pass type checks. + */ export function useMutationForm< FormShape extends FieldValues = never, TMutation extends CommonMutationParameters = never, From 3e6a05aa5f10de6a7aee7fc30644d63c1b2f4d38 Mon Sep 17 00:00:00 2001 From: Vyacheslav Matyukhin Date: Fri, 29 Sep 2023 21:38:41 -0600 Subject: [PATCH 4/4] preparing for re-merge from main --- packages/hub/src/app/new/model/NewModel.tsx | 36 ++++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/packages/hub/src/app/new/model/NewModel.tsx b/packages/hub/src/app/new/model/NewModel.tsx index 662485125a..fa2a25568e 100644 --- a/packages/hub/src/app/new/model/NewModel.tsx +++ b/packages/hub/src/app/new/model/NewModel.tsx @@ -1,7 +1,7 @@ "use client"; import { useSession } from "next-auth/react"; -import { useRouter } from "next/navigation"; -import { FC } from "react"; +import { useRouter, useSearchParams } from "next/navigation"; +import { FC, useEffect } from "react"; import { FormProvider } from "react-hook-form"; import { graphql } from "relay-runtime"; @@ -13,7 +13,9 @@ import { SelectGroup, SelectGroupOption } from "@/components/SelectGroup"; import { H1 } from "@/components/ui/Headers"; import { SlugFormField } from "@/components/ui/SlugFormField"; import { useMutationForm } from "@/hooks/useMutationForm"; -import { modelRoute } from "@/routes"; +import { modelRoute, newModelRoute } from "@/routes"; +import { useLazyLoadQuery } from "react-relay"; +import { NewModelPageQuery } from "@/__generated__/NewModelPageQuery.graphql"; const defaultCode = `/* Describe your code here @@ -31,7 +33,32 @@ type FormShape = { export const NewModel: FC = () => { useSession({ required: true }); + const searchParams = useSearchParams(); + + const { group: initialGroup } = useLazyLoadQuery( + graphql` + query NewModelPageQuery($groupSlug: String!, $groupSlugIsSet: Boolean!) { + group(slug: $groupSlug) @include(if: $groupSlugIsSet) { + ... on Group { + id + slug + myMembership { + id + } + } + } + } + `, + { + groupSlug: searchParams.get("group") ?? "", + groupSlugIsSet: Boolean(searchParams.get("group")), + } + ); + const router = useRouter(); + useEffect(() => { + router.replace(newModelRoute()); // clean up group=... param + }, [router]); const { form, onSubmit, inFlight } = useMutationForm< FormShape, @@ -41,7 +68,7 @@ export const NewModel: FC = () => { mode: "onChange", defaultValues: { // don't pass `slug: ""` here, it will lead to form reset if a user started to type in a value before JS finished loading - group: null, + group: initialGroup?.myMembership ? initialGroup : null, isPrivate: false, }, mutation: graphql` @@ -99,6 +126,7 @@ export const NewModel: FC = () => { /> label="Group" + description="Optional. Models owned by a group are editable by all members of the group." name="group" required={false} myOnly={true}